From 5287368dcd3c4a7befb6b3b55575239183bed236 Mon Sep 17 00:00:00 2001 From: Odin Hultgren Van Der Horst Date: Fri, 5 Nov 2021 08:48:07 +0100 Subject: [PATCH] Requested changes - Split comparison ifs into two seperate ifs for both cog and cas - Changed variable name for shdict cog from match to unmatch - Added comment to "only set flag" branch in shdict store - Removed some unesesary blank lines - Moved forcefull alloc for shdict store and cas into a seperate function - Removed unsued function "ngx_http_lua_ffi_shdict_get_conditonal" --- src/ngx_http_lua_shdict.c | 292 +++++++++++--------------------------- 1 file changed, 81 insertions(+), 211 deletions(-) diff --git a/src/ngx_http_lua_shdict.c b/src/ngx_http_lua_shdict.c index 0cc90ae03d..6ca3f79bd8 100644 --- a/src/ngx_http_lua_shdict.c +++ b/src/ngx_http_lua_shdict.c @@ -305,6 +305,37 @@ ngx_http_lua_shdict_expire(ngx_http_lua_shdict_ctx_t *ctx, ngx_uint_t n) } +static inline ngx_rbtree_node_t * +ngx_http_lua_shdict_force_alloc(ngx_http_lua_shdict_ctx_t *ctx, size_t size, + int *forcible) +{ + int i; + ngx_rbtree_node_t *node; + + node = ngx_slab_alloc_locked(ctx->shpool, size); + *forcible = 0; + + if (node) { + return node; + } + + for (i = 0; i < 30; i++) { + if (ngx_http_lua_shdict_expire(ctx, 0) == 0) { + break; + } + + *forcible = 1; + + node = ngx_slab_alloc_locked(ctx->shpool, size); + if (node) { + return node; + } + } + + return NULL; +} + + void ngx_http_lua_inject_shdict_api(ngx_http_lua_main_conf_t *lmcf, lua_State *L) { @@ -1313,7 +1344,7 @@ ngx_http_lua_ffi_shdict_cas(ngx_shm_zone_t *zone, u_char *key, double num_value, int user_flags, int set_user_flags, long exptime, int match_flags, int *match, char **errmsg, int *forcible) { - int i, n, match_value; + int n, match_value; u_char c, oc, *p; uint32_t hash; ngx_int_t rc; @@ -1401,13 +1432,22 @@ ngx_http_lua_ffi_shdict_cas(ngx_shm_zone_t *zone, u_char *key, dd("lookup returns %d", (int) rc); - if ((match_flags || match_value) && - ((rc == NGX_DECLINED || rc == NGX_DONE) || - (match_flags && old_user_flags != (int) sd->user_flags) || - (match_value && (old_value_type != sd->value_type || - old_str_value_len != sd->value_len || - ngx_memcmp(old_str_value_buf, sd->data + key_len, - (size_t) old_str_value_len))))) + + if (match_flags + && (rc != NGX_OK || old_user_flags != (int) sd->user_flags)) + { + ngx_shmtx_unlock(&ctx->shpool->mutex); + *match = 1; + return NGX_DECLINED; + } + + if (match_value + && (rc != NGX_OK + || old_value_type != sd->value_type + || old_str_value_len != sd->value_len + || ngx_memcmp(old_str_value_buf, sd->data + key_len, + (size_t) old_str_value_len) + )) { ngx_shmtx_unlock(&ctx->shpool->mutex); *match = 1; @@ -1420,6 +1460,7 @@ ngx_http_lua_ffi_shdict_cas(ngx_shm_zone_t *zone, u_char *key, } if (set_user_flags && value_type == LUA_TNIL) { + /* Only set flag, leave value unchanged */ ngx_queue_remove(&sd->queue); ngx_queue_insert_head(&ctx->sh->lru_queue, &sd->queue); @@ -1451,7 +1492,6 @@ ngx_http_lua_ffi_shdict_cas(ngx_shm_zone_t *zone, u_char *key, ngx_queue_remove(&sd->queue); ngx_queue_insert_head(&ctx->sh->lru_queue, &sd->queue); - if (exptime > 0) { tp = ngx_timeofday(); sd->expires = (uint64_t) tp->sec * 1000 + tp->msec @@ -1508,7 +1548,6 @@ ngx_http_lua_ffi_shdict_cas(ngx_shm_zone_t *zone, u_char *key, ngx_rbtree_delete(&ctx->sh->rbtree, node); ngx_slab_free_locked(ctx->shpool, node); - } /* rc == NGX_DECLINED or value size unmatch */ @@ -1526,35 +1565,21 @@ ngx_http_lua_ffi_shdict_cas(ngx_shm_zone_t *zone, u_char *key, + key_len + str_value_len; - node = ngx_slab_alloc_locked(ctx->shpool, n); - - if (node == NULL) { + node = ngx_http_lua_shdict_force_alloc(ctx, n, forcible); + if (*forcible == 1) { ngx_log_debug2(NGX_LOG_DEBUG_HTTP, ctx->log, 0, - "lua shared dict cas: overriding non-expired items " + "lua shared dict cas: overwrote non-expired items " "due to memory shortage for entry \"%*s\"", key_len, key); + } - for (i = 0; i < 30; i++) { - if (ngx_http_lua_shdict_expire(ctx, 0) == 0) { - break; - } - - *forcible = 1; - - node = ngx_slab_alloc_locked(ctx->shpool, n); - if (node != NULL) { - goto allocated; - } - } - + if (!node) { ngx_shmtx_unlock(&ctx->shpool->mutex); *errmsg = "no memory"; return NGX_ERROR; } -allocated: - sd = (ngx_http_lua_shdict_node_t *) &node->color; node->key = hash; @@ -1591,7 +1616,7 @@ ngx_http_lua_ffi_shdict_cog(ngx_shm_zone_t *zone, u_char *old_str_value_buf, size_t old_str_value_len, double old_num_value, int old_user_flags, int *value_type, u_char **str_value_buf, size_t *str_value_len, double *num_value, - int *user_flags, int match_flags, int *match, char **errmsg) + int *user_flags, int match_flags, int *unmatch, char **errmsg) { u_char oc; ngx_str_t name; @@ -1603,7 +1628,7 @@ ngx_http_lua_ffi_shdict_cog(ngx_shm_zone_t *zone, int match_value; *errmsg = NULL; - *match = 0; + *unmatch = 0; ctx = zone->data; name = ctx->name; @@ -1661,18 +1686,19 @@ ngx_http_lua_ffi_shdict_cog(ngx_shm_zone_t *zone, return NGX_OK; } - - /* rc == NGX_OK || (rc == NGX_DONE) */ - if ((match_value || match_flags) && - (!match_flags || old_user_flags == (int) sd->user_flags) && - (!match_value || (old_value_type == sd->value_type && - old_str_value_len == sd->value_len && - !ngx_memcmp(old_str_value_buf, sd->data + key_len, - (size_t) old_str_value_len)))) - { - ngx_shmtx_unlock(&ctx->shpool->mutex); - *match = 1; - return NGX_DECLINED; + if (match_value || match_flags) { + if (!match_flags || old_user_flags == (int) sd->user_flags) + { + if (!match_value || (old_value_type == sd->value_type && + old_str_value_len == sd->value_len && + !ngx_memcmp(old_str_value_buf, sd->data + key_len, + (size_t) old_str_value_len))) + { + ngx_shmtx_unlock(&ctx->shpool->mutex); + *unmatch = 1; + return NGX_DECLINED; + } + } } *value_type = sd->value_type; @@ -1766,7 +1792,7 @@ ngx_http_lua_ffi_shdict_store(ngx_shm_zone_t *zone, int op, u_char *key, size_t str_value_len, double num_value, long exptime, int user_flags, char **errmsg, int *forcible) { - int i, n; + int n; u_char c, *p; uint32_t hash; ngx_int_t rc; @@ -1952,43 +1978,26 @@ ngx_http_lua_ffi_shdict_store(ngx_shm_zone_t *zone, int op, u_char *key, + key_len + str_value_len; - node = ngx_slab_alloc_locked(ctx->shpool, n); - - if (node == NULL) { - - if (op & NGX_HTTP_LUA_SHDICT_SAFE_STORE) { - ngx_shmtx_unlock(&ctx->shpool->mutex); - - *errmsg = "no memory"; - return NGX_ERROR; - } - - ngx_log_debug2(NGX_LOG_DEBUG_HTTP, ctx->log, 0, - "lua shared dict set: overriding non-expired items " - "due to memory shortage for entry \"%*s\"", key_len, - key); - - for (i = 0; i < 30; i++) { - if (ngx_http_lua_shdict_expire(ctx, 0) == 0) { - break; - } - - *forcible = 1; + if (op & NGX_HTTP_LUA_SHDICT_SAFE_STORE) { + node = ngx_slab_alloc_locked(ctx->shpool, n); - node = ngx_slab_alloc_locked(ctx->shpool, n); - if (node != NULL) { - goto allocated; - } + } else { + node = ngx_http_lua_shdict_force_alloc(ctx, n, forcible); + if (*forcible) { + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, ctx->log, 0, + "lua shared dict store: overwrote non-expired items " + "due to memory shortage for entry \"%*s\"", key_len, + key); } + } + if (!node) { ngx_shmtx_unlock(&ctx->shpool->mutex); *errmsg = "no memory"; return NGX_ERROR; } -allocated: - sd = (ngx_http_lua_shdict_node_t *) &node->color; node->key = hash; @@ -2158,145 +2167,6 @@ ngx_http_lua_ffi_shdict_get(ngx_shm_zone_t *zone, u_char *key, } -int -ngx_http_lua_ffi_shdict_get_conditonal(ngx_shm_zone_t *zone, u_char *key, - size_t key_len, int *value_type, u_char **str_value_buf, - size_t *str_value_len, double *num_value, int *user_flags, - int get_stale, int *is_stale, char **err) -{ - ngx_str_t name; - uint32_t hash; - ngx_int_t rc; - ngx_http_lua_shdict_ctx_t *ctx; - ngx_http_lua_shdict_node_t *sd; - ngx_str_t value; - - *err = NULL; - - ctx = zone->data; - name = ctx->name; - - hash = ngx_crc32_short(key, key_len); - -#if (NGX_DEBUG) - ngx_log_debug3(NGX_LOG_DEBUG_HTTP, ctx->log, 0, - "fetching key \"%*s\" in shared dict \"%V\"", key_len, - key, &name); -#endif /* NGX_DEBUG */ - - ngx_shmtx_lock(&ctx->shpool->mutex); - -#if 1 - if (!get_stale) { - ngx_http_lua_shdict_expire(ctx, 1); - } -#endif - - rc = ngx_http_lua_shdict_lookup(zone, hash, key, key_len, &sd); - - dd("shdict lookup returns %d", (int) rc); - - if (rc == NGX_DECLINED || (rc == NGX_DONE && !get_stale)) { - ngx_shmtx_unlock(&ctx->shpool->mutex); - *value_type = LUA_TNIL; - return NGX_OK; - } - - /* rc == NGX_OK || (rc == NGX_DONE && get_stale) */ - - *value_type = sd->value_type; - - dd("data: %p", sd->data); - dd("key len: %d", (int) sd->key_len); - - value.data = sd->data + sd->key_len; - value.len = (size_t) sd->value_len; - - if (*str_value_len < (size_t) value.len) { - if (*value_type == SHDICT_TBOOLEAN) { - ngx_shmtx_unlock(&ctx->shpool->mutex); - return NGX_ERROR; - } - - if (*value_type == SHDICT_TSTRING) { - *str_value_buf = malloc(value.len); - if (*str_value_buf == NULL) { - ngx_shmtx_unlock(&ctx->shpool->mutex); - return NGX_ERROR; - } - } - } - - switch (*value_type) { - - case SHDICT_TSTRING: - *str_value_len = value.len; - ngx_memcpy(*str_value_buf, value.data, value.len); - break; - - case SHDICT_TNUMBER: - - if (value.len != sizeof(double)) { - ngx_shmtx_unlock(&ctx->shpool->mutex); - ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0, - "bad lua number value size found for key %*s " - "in shared_dict %V: %z", key_len, key, - &name, value.len); - return NGX_ERROR; - } - - *str_value_len = value.len; - ngx_memcpy(num_value, value.data, sizeof(double)); - break; - - case SHDICT_TBOOLEAN: - - if (value.len != sizeof(u_char)) { - ngx_shmtx_unlock(&ctx->shpool->mutex); - ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0, - "bad lua boolean value size found for key %*s " - "in shared_dict %V: %z", key_len, key, &name, - value.len); - return NGX_ERROR; - } - - ngx_memcpy(*str_value_buf, value.data, value.len); - break; - - case SHDICT_TLIST: - - ngx_shmtx_unlock(&ctx->shpool->mutex); - - *err = "value is a list"; - return NGX_ERROR; - - default: - - ngx_shmtx_unlock(&ctx->shpool->mutex); - ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0, - "bad value type found for key %*s in " - "shared_dict %V: %d", key_len, key, &name, - *value_type); - return NGX_ERROR; - } - - *user_flags = sd->user_flags; - dd("user flags: %d", *user_flags); - - ngx_shmtx_unlock(&ctx->shpool->mutex); - - if (get_stale) { - - /* always return value, flags, stale */ - - *is_stale = (rc == NGX_DONE); - return NGX_OK; - } - - return NGX_OK; -} - - 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,