feat: implements timeout for keyval

thiagoc01 commented 8 months ago

Using the timer for events implemented by nginx, we can use ttl in keyval zones. This creates a struct for store the pointer for the node, which should be deleted after an amount of time specified on the configuration, and the pointer for the context. The ttl belongs to this context too. When the node is created, the function ngx_keyval_shm_set_data creates an event and adds a timer for this event. The handler (ngx_keyval_delete_timeout_node_shm) deletes the node. So, the next time when key is checked, there won't be a node with it.

Summary by CodeRabbit

New Features
- Added inclusion of ngx_event.h for improved event handling.
- Implemented TTL configuration for automatic expiration of key-value pairs.
- Introduced parsing and handling of variables in strings for dynamic content.
- Enhanced shared memory functionality to manage timeout nodes effectively.
Improvements
- Optimized memory management and error handling in key-value string construction.
- Extended key-value storage to support multiple indexes for complex data scenarios.
Documentation
- Updated to include details on TTL configuration, variable handling, and memory management changes.

coderabbitai[bot] commented 8 months ago

Walkthrough

The recent updates to the Nginx module introduce enhanced variable handling and TTL-based control for key-value storage. The changes focus on dynamic memory allocation for constructing strings with replaced variables and incorporate timeout mechanisms for data retention. This overhaul facilitates more flexible and efficient data manipulation and storage, accommodating complex configurations and ensuring data freshness through TTL settings.

Changes

Files	Change Summaries
`.../ngx_http_keyval_module.c` `.../ngx_stream_keyval_module.c`	Updated to handle multiple variables for replacement in strings, including dynamic memory allocation and construction of the final string with replaced variables.
`src/ngx_keyval.c` `src/ngx_keyval.h`	Added inclusion of `ngx_event.h`. Implemented TTL parsing and handling, introduced `ngx_keyval_node_timeout_t` structure, and updated `ngx_keyval_shm_ctx_t` for timeout management in shared memory. Enhanced handling of multiple indexes for variable values.
`t/keyval.t` `t/keyval_multivar.t`	Introduce different timeout scenarios with varying durations, affecting key-value data retrieval and setting operations within specified time limits. Added tests to ensure correct data retrieval and storage, including handling of additional characters and variable existence.

🐰✨
In the realm of web, under digital skies,
A hop, a leap, through code and ties.
Variables dance, memory aligns,
With TTL whispers, time confines.
Through tests and trials, the changes gleam,
A rabbit's work, a coder's dream.
🌟📜

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips

### Chat There are 3 ways to chat with CodeRabbit: - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit-tests for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit tests for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit tests.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - The JSON schema for the configuration file is available [here](https://coderabbit.ai/integrations/coderabbit-overrides.v2.json). - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json` ### CodeRabbit Discord Community Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback.

kjdev commented 8 months ago

[!CAUTION] nginx: [emerg] invalid number of arguments in "keyval_zone" directive

Please correct.

--- a/src/ngx_http_keyval_module.c
+++ b/src/ngx_http_keyval_module.c
@@ -18,7 +18,7 @@ static ngx_command_t ngx_http_keyval_commands[] = {
     0,
     NULL },
   { ngx_string("keyval_zone"),
-    NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
+    NGX_HTTP_MAIN_CONF|NGX_CONF_1MORE,
     ngx_http_keyval_conf_set_zone,
     0,
     0,
--- a/src/ngx_stream_keyval_module.c
+++ b/src/ngx_stream_keyval_module.c
@@ -18,7 +18,7 @@ static ngx_command_t ngx_stream_keyval_commands[] = {
     0,
     NULL },
   { ngx_string("keyval_zone"),
-    NGX_STREAM_MAIN_CONF|NGX_CONF_TAKE1,
+    NGX_STREAM_MAIN_CONF|NGX_CONF_1MORE,
     ngx_stream_keyval_conf_set_zone,
     0,
     0,

kjdev commented 8 months ago

Fixing code styles.

--- a/src/ngx_keyval.c
+++ b/src/ngx_keyval.c
@@ -278,16 +278,15 @@ ngx_keyval_conf_set_zone(ngx_conf_t *cf, ngx_command_t *cmd, void *conf,
       ctx->ttl = ngx_parse_time(&s, 1);
       if (ctx->ttl == (time_t) NGX_ERROR) {
         ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
-                          "\"%V\" invalid ttl \"%V\"",
-                          &cmd->name, &value[2]);
+                           "\"%V\" invalid ttl \"%V\"",
+                           &cmd->name, &value[2]);
         ctx->ttl = 0;
         return NGX_CONF_ERROR;
       }
     }
-  }
-
-  else
+  } else {
     ctx->ttl = 0;
+  }

   return NGX_CONF_OK;
 }
@@ -575,7 +574,8 @@ ngx_keyval_shm_get_data(ngx_keyval_shm_ctx_t *ctx, ngx_shm_zone_t *shm,
 static void
 ngx_keyval_delete_timeout_node_shm(ngx_event_t *node_status)
 {
-  ngx_keyval_node_timeout_t *arg = (ngx_keyval_node_timeout_t *) node_status->data;
+  ngx_keyval_node_timeout_t *arg
+    = (ngx_keyval_node_timeout_t *) node_status->data;

   if (arg->ctx->shpool != NULL && arg->node != NULL) {
     ngx_rbtree_delete(&arg->ctx->sh->rbtree, arg->node);
@@ -632,25 +632,24 @@ ngx_keyval_shm_set_data(ngx_keyval_shm_ctx_t *ctx, ngx_shm_zone_t *shm,
     rc = NGX_OK;

     if (ctx->ttl) {
-      ngx_event_t *timeout_node_event = ngx_slab_alloc_locked(ctx->shpool, sizeof(ngx_event_t));
+      ngx_event_t *timeout_node_event
+        = ngx_slab_alloc_locked(ctx->shpool, sizeof(ngx_event_t));

       if (timeout_node_event == NULL) {
         ngx_log_error(NGX_LOG_ERR, log, 0,
                       "keyval: failed to allocate timeout event");
         rc = NGX_ERROR;
-      }
-
-      else {
-        ngx_keyval_node_timeout_t *timeout_node = ngx_slab_alloc_locked(ctx->shpool, sizeof(ngx_keyval_node_timeout_t));
+      } else {
+        ngx_keyval_node_timeout_t *timeout_node
+          = ngx_slab_alloc_locked(ctx->shpool,
+                                  sizeof(ngx_keyval_node_timeout_t));

         if (timeout_node == NULL) {
           ngx_log_error(NGX_LOG_ERR, log, 0,
-                        "keyval:failed to allocate timeout node");
+                        "keyval: failed to allocate timeout node");
           rc = NGX_ERROR;
           ngx_slab_free_locked(ctx->shpool, timeout_node_event);
-        }
-
-        else {
+        } else {
           timeout_node->node = node;
           timeout_node->ctx = ctx;

kjdev commented 8 months ago

Please add a ttl test to the stream module as well.

t/stream.t

kjdev commented 8 months ago

Please update README.md as well.

thiagoc01 commented 8 months ago

[!CAUTION] nginx: [emerg] invalid number of arguments in "keyval_zone" directive

Please correct.

--- a/src/ngx_http_keyval_module.c
+++ b/src/ngx_http_keyval_module.c
@@ -18,7 +18,7 @@ static ngx_command_t ngx_http_keyval_commands[] = {
     0,
     NULL },
   { ngx_string("keyval_zone"),
-    NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
+    NGX_HTTP_MAIN_CONF|NGX_CONF_1MORE,
     ngx_http_keyval_conf_set_zone,
     0,
     0,
--- a/src/ngx_stream_keyval_module.c
+++ b/src/ngx_stream_keyval_module.c
@@ -18,7 +18,7 @@ static ngx_command_t ngx_stream_keyval_commands[] = {
     0,
     NULL },
   { ngx_string("keyval_zone"),
-    NGX_STREAM_MAIN_CONF|NGX_CONF_TAKE1,
+    NGX_STREAM_MAIN_CONF|NGX_CONF_1MORE,
     ngx_stream_keyval_conf_set_zone,
     0,
     0,

I'm testing in a container without volumes, and coding in host. I changed in container, but not in host and I forgot 😞😞

kjdev commented 8 months ago

The original http_keyval_module of nginx can be set with timeout=time, so is it possible to set it with timeout= as well?

I don't consider this a requirement, so please just reply.

If not, I will fix it after the merge.

thiagoc01 commented 8 months ago

It's just change the argument name, right?

kjdev commented 8 months ago

Yes, just return it. I was wondering if either ttl or timeout could be defined.

thiagoc01 commented 8 months ago

We could leave like this too, it's inspired in the module and the use of "ttl" is documented. But if you want, it's easy, we just insert an "or" in the parse of configuration.

kjdev commented 7 months ago

I don't see any major problems.

I'd like to make a few adjustments and then merge. https://github.com/kjdev/nginx-keyval/commits/thiagoc01-timeout-keyval-zone/

kjdev commented 7 months ago

merged https://github.com/kjdev/nginx-keyval/commit/0adec5c992368fd0c4c06904dc76eba9da854b4a

kjdev / nginx-keyval