Incr method exptime

[splitice]

Added expttime to incr, the behaviour of this pull request matches that of my pending pull request for the lua module

[agentzh]

@splitice Thanks for your contribution! I'll look into this :)

[doujiang24]

@splitice I think it better to let the second option arg to be init (the init value when the key is not exists), like: incr(key, init). You can take a look at: openresty/lua-nginx-module#579

May be we can add expire command?
openresty/lua-nginx-module#586

[splitice]

option arg - I dont disagree that its possibly a good idea. I am however not sure if I could port that to the C version of the module. I'll wat for @agentzh's input first though.

expire command - personally I've found in lots of work that I have done the requirement to increment and "bump" up the expire time to be extremely common, especially in counters or limits.

An expire command is probably a good idea, I do however like it in the incr, it also makes handling possible race conditions easier :)

[splitice]

Oh wait, thats a pull request. 👍

No code requirement from me :)

[doujiang24]

2016-01-04 12:25 GMT+08:00 Mathew Heard notifications@github.com:

option arg - I dont disagree that its possibly a good idea. I am however
not sure if I could port that to the C version of the module. I'll wat for
@agentzh https://github.com/agentzh's input first though.

expire command - personally I've found in lots of work that I have done
the requirement to increment and "bump" up the expire time to be extremely
common, especially in counters or limits.

Yeah, same as incr(key, init)

An expire command is probably a good idea, I do however like it in the
incr, it also makes handling possible race conditions easier :)

Yeah, I agree, it's much easier to set expire in incr.
If we have to choose one, I personally like incr(key, init) than
incr(key, expt). ( I personally like redis style )
Or it can be incr(key, init, expt) or incr(key, expt, init) ?

—
Reply to this email directly or view it on GitHub
#10 (comment)
.

[agentzh]

I'm fine with incr(key, value, init, expire). And I also want to have a separate expire(time) method :)

[splitice]

personally if you went the init path I would prefer an options style array, at that point I feel the method is getting a bit to complex with optional fields, that also leaves room for more expansion.

Such as:

incr(key,value,options:{expire:,init:})

[agentzh]

@splitice I understand that concern. Options tables are generally more expensive than passed-by-position arguments though :)

[splitice]

True, And I certainly agree since I use incr in some performance sensitive paths.

One other point, I would probably put init fourth. Popularity wise I would expect it to be less used.

[agentzh]

@splitice Personally I use init much more often ;) But I agree it can be more consistent with the set method's parameter order.

[splitice]

One more quick fix applied, the default value is inconsistent between resty core and the shared dict. Defaulted it to one for compatibility.