Skip to content

ngx.shared.DICT.incr optional expire argument #942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

ngx.shared.DICT.incr optional expire argument #942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a4640ce
incr expire adrgument
splitice Dec 29, 2016
7c4514f
fix init path
splitice Dec 29, 2016
2e760ec
fix > 80 lines
splitice Dec 30, 2016
3aa45d4
style
splitice Dec 30, 2016
6207e36
style
splitice Dec 30, 2016
285c498
Style: add newline
splitice Jan 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 71 additions & 11 deletions src/ngx_http_lua_shdict.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1257,25 +1257,54 @@ ngx_http_lua_shdict_incr(lua_State *L)
ngx_http_lua_shdict_node_t *sd;
double num;
double init = 0;
int has_init = 0;
u_char *p;
ngx_shm_zone_t *zone;
double value;
lua_Number exptime = -1;
ngx_rbtree_node_t *node;
/* indicates whether to foricibly override other
* valid entries */
int forcible = 0;
ngx_queue_t *queue, *q;
ngx_time_t *tp;

n = lua_gettop(L);

if (n != 3 && n != 4) {
return luaL_error(L, "expecting 3 or 4 arguments, but only seen %d", n);
if (n < 3) {
return luaL_error(L, "expecting at least 3 arguments, but only seen %d",
n);
}

if (n > 5) {
return luaL_error(L, "expecting no more than 5 arguments, but %d seen",
n);
}

if (lua_type(L, 1) != LUA_TTABLE) {
return luaL_error(L, "bad \"zone\" argument");
}

if (n >= 4) {
if (!lua_isnil(L, 4)) {
init = luaL_checknumber(L, 4);
if (init < 0) {
return luaL_error(L, "bad \"init\" argument");
}

has_init = 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: need a blank line after the } line.

}

if (n >= 5) {
if (!lua_isnil(L, 5)) {
exptime = luaL_checknumber(L, 5);
if (exptime < 0) {
return luaL_error(L, "bad \"exptime\" argument");
}
}
}
}

zone = ngx_http_lua_shdict_get_zone(L, 1);
if (zone == NULL) {
return luaL_error(L, "bad user data for the ngx_shm_zone_t pointer");
Expand Down Expand Up @@ -1307,10 +1336,6 @@ ngx_http_lua_shdict_incr(lua_State *L)

value = luaL_checknumber(L, 3);

if (n == 4) {
init = luaL_checknumber(L, 4);
}

dd("looking up key %.*s in shared dict %.*s", (int) key.len, key.data,
(int) ctx->name.len, ctx->name.data);

Expand All @@ -1326,7 +1351,7 @@ ngx_http_lua_shdict_incr(lua_State *L)

if (rc == NGX_DECLINED || rc == NGX_DONE) {

if (n == 3) {
if (!has_init) {
ngx_shmtx_unlock(&ctx->shpool->mutex);

lua_pushnil(L);
Expand Down Expand Up @@ -1376,6 +1401,18 @@ ngx_http_lua_shdict_incr(lua_State *L)
ngx_queue_remove(&sd->queue);
ngx_queue_insert_head(&ctx->sh->lru_queue, &sd->queue);

if (exptime > 0) {
dd("setting expire time to %d", exptime);

tp = ngx_timeofday();
sd->expires = (uint64_t) tp->sec * 1000 + tp->msec
+ (uint64_t) (exptime * 1000);

} else if (exptime == 0) {
dd("setting key to never expire");
sd->expires = 0;
}

dd("setting value type to %d", (int) sd->value_type);

p = sd->data + key.len;
Expand Down Expand Up @@ -1476,9 +1513,19 @@ ngx_http_lua_shdict_incr(lua_State *L)

setvalue:

sd->user_flags = 0;
if (exptime > 0) {
dd("setting expire time to %d", exptime);

sd->expires = 0;
tp = ngx_timeofday();
sd->expires = (uint64_t) tp->sec * 1000 + tp->msec
+ (uint64_t) (exptime * 1000);

} else {
dd("setting key to never expire");
sd->expires = 0;
}

sd->user_flags = 0;

dd("setting value type to %d", LUA_TNUMBER);

Expand Down Expand Up @@ -2622,8 +2669,8 @@ ngx_http_lua_ffi_shdict_get(ngx_shm_zone_t *zone, u_char *key,

int
ngx_http_lua_ffi_shdict_incr(ngx_shm_zone_t *zone, u_char *key,
size_t key_len, double *value, char **err, int has_init, double init,
int *forcible)
size_t key_len, double *value, int exptime, char **err, int has_init,
double init, int *forcible)
{
int i, n;
uint32_t hash;
Expand All @@ -2633,6 +2680,7 @@ ngx_http_lua_ffi_shdict_incr(ngx_shm_zone_t *zone, u_char *key,
double num;
ngx_rbtree_node_t *node;
u_char *p;
ngx_time_t *tp;
ngx_queue_t *queue, *q;

if (zone == NULL) {
Expand Down Expand Up @@ -2712,6 +2760,18 @@ ngx_http_lua_ffi_shdict_incr(ngx_shm_zone_t *zone, u_char *key,

ngx_memcpy(p, (double *) &num, sizeof(double));

if (exptime > 0) {
dd("setting expire time to %d", exptime);

tp = ngx_timeofday();
sd->expires = (uint64_t) tp->sec * 1000 + tp->msec
+ (uint64_t) (exptime * 1000);

} else if (exptime == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better make exptime < 0 turn into exptime == 0 as well. So better replace this else if with else directly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are different code paths. Not adjusting the exptime (<0 aka -1 the default for null) vs setting it to not expire (0)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@splitice Okay, got it.

dd("setting key to never expire");
sd->expires = 0;
}

ngx_shmtx_unlock(&ctx->shpool->mutex);

*value = num;
Expand Down
87 changes: 87 additions & 0 deletions t/043-shdict.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2471,3 +2471,90 @@ error
lua_shared_dict "dogs" is already defined as "dogs"
--- error_log
[emerg]



=== TEST 94: incr expire test new key
--- http_config
lua_shared_dict dogs 1m;
--- config
location = /test {
content_by_lua '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newly added test cases should use the *_by_lua_block form instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do a future PR changing all the shdict tests to _block, not just these ones. It's an easy regex.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@splitice I prefer keeping the old tests intact. Just the newly added ones should use *_by_lua_block.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to make adjustments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@splitice You want me to make adjustments in your branch? Sorry, no, I can't, obviously :)

Copy link
Member

@agentzh agentzh Jan 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@splitice We can only help with the review process since we have many other items with much higher priorities. Thanks for your understanding.

local dogs = ngx.shared.dogs
dogs:incr("foo", 32, 1, 0.5)
ngx.sleep(0.001)
ngx.say(dogs:get("foo"))
';
}
--- request
GET /test
--- response_body
33
--- no_error_log
[error]



=== TEST 95: incr expire test new key expire
--- http_config
lua_shared_dict dogs 1m;
--- config
location = /test {
content_by_lua '
local dogs = ngx.shared.dogs
dogs:incr("foo", 32, 1, 0.01)
ngx.sleep(0.02)
ngx.say(dogs:get("foo"))
';
}
--- request
GET /test
--- response_body
nil
--- no_error_log
[error]



=== TEST 96: incr expire test existing key expire
--- http_config
lua_shared_dict dogs 1m;
--- config
location = /test {
content_by_lua '
local dogs = ngx.shared.dogs
dogs:incr("foo", 32, 1)
dogs:incr("foo", 32, nil, 0.01)
ngx.sleep(0.02)
ngx.say(dogs:get("foo"))
';
}
--- request
GET /test
--- response_body
nil
--- no_error_log
[error]



=== TEST 97: incr expire set no init
--- http_config
lua_shared_dict dogs 1m;
--- config
location = /test {
content_by_lua '
local dogs = ngx.shared.dogs
dogs:incr("foo", 32, nil, 0.5)
ngx.sleep(0.01)
ngx.say(dogs:get("foo"))
';
}
--- request
GET /test
--- response_body
nil
--- no_error_log
[error]