Will shared-memory be implemented or considered?

[HermannLizard]

https://github.com/nginx/njs/issues/431 https://github.com/nginx/njs/issues/354#issuecomment-723712610

For instance, this can be used in web gray-release solution, totally replace the "redis+nginx+lua" solution, since web maintainers is usually more familiar with JavaScript than lua, like, me. More controllable, more flexible. Sincerely looking forward to it.

[xeioex]

Hi @HermannLizard,

We are planning to open source keyval module for this.

[chs97]

@xeioex Are there any updates

[raoajayy]

@xeioex Any timeline to open source keyval module

[xeioex]

Before keyval module is release, I plan to add a shared dictionary to njs.

[xeioex]

Hello everyone,

@HermannLizard, @osokin, @drsm, @ivanitskiy, @crasyangel, @FlorianSW, @7rulnik, @bfuvx, @cxpcyv, @jirutka and others feel free to test and give feedback for upcoming shared dictionary feature.

https://gist.github.com/xeioex/b61de2bba239cc84bf7c459966367ddf

Modules: introduced js_shared_dict_zone directive.

The directive allows to declared a dictionary that is shared among the
working processes. A dictonary expects strings as keys. It stores
string or number as values. The value type is declared using
type= argument of the direcive. The default type is string.

For example:
    nginx.conf:
        # Declares a shared dictionary of strings of size 1 Mb, that
        # removes key-value after 60 seconds of inactivity.
        js_shared_dict_zone zone=foo:1M timeout=60s;
        # Declares a shared dictionary of strings of size 512Kb, that
        # forsibly remove oldest key-value pairs when memory is not enough.
        js_shared_dict_zone zone=bar:512K timeout=30s evict;
        # Declares a permanent number shared dictionary of size 32Kb.
        js_shared_dict_zone zone=num:32k type=number;

    test.js:
        function get(r) {
            r.return(200, ngx.shared.foo.get(r.args.key));

        function set(r) {
            r.return(200, ngx.shared.foo.set(r.args.key, r.args.value));
        }

        function delete(r) {
            r.return(200, ngx.shared.bar.delete(r.args.key));
        }

        function increment(r) {
            r.return(200, ngx.shared.num.incr(r.args.key, 1));
        }

[drsm]

maybe freeSpace ? typo

[hongzhidao]

Hi Dmitry,

1. About the commit log.

   https://gist.github.com/xeioex/357e64b871608cae08f8fc7fb184b151#file-shared_dict-patch-L9
   s/The directives allows/The directive allows/
   
   test.js:
       function get(r) {
           r.return(200, ngx.shared.foo.get(r.args.key));
      // missed } here 
   
       function set(r) {
           r.return(200, ngx.shared.foo.set(r.args.key, r.args.value));
       }

2. What's the intention of the usage?

   js_shared_dict_zone zone=bar:512K timeout=30s evict;
   js_shared_dict_zone zone=baz:512K timeout=30s evict;
   
   var dict = ngx.shared.baz;
   dict.set("foo", "baz");
   
   r.return(200, ngx.shared.get("foo"));

   The ngx.shared.get is the same as ngx.shared.baz.get, since the last zone was regarded as the default shared dict. Is it your intention? Is it because of the convenience for users?

3. Handing the case of empty value. a. missed a test on it. b. the size of value->length is 0 for both empty value and delete operation, so using value->start == NULL for the delete operation.

   
   diff -r 3280c24bba68 nginx/ngx_shared_dict.c
   --- a/nginx/ngx_shared_dict.c   Fri Jun 09 00:08:46 2023 -0700
   +++ b/nginx/ngx_shared_dict.c   Mon Jun 12 18:22:21 2023 +0800
   @@ -313,7 +313,7 @@
    node = ngx_js_dict_lookup(dict, key);
   
    if (node == NULL) {
   -        if (value->length == 0) {
   +        if (value->start == NULL) {
            ngx_rwlock_unlock(&dict->sh->rwlock);
            return NGX_OK;
        }
   @@ -325,7 +325,7 @@
            return NGX_ERROR;
        }

• } else if (value->length == 0) {

• } else if (value->start == NULL) { ngx_js_dict_delete(dict, node);

  } else {

4. Just an idea to simplify related allocation and free.

   
   diff -r 3280c24bba68 nginx/ngx_shared_dict.c
   --- a/nginx/ngx_shared_dict.c   Fri Jun 09 00:08:46 2023 -0700
   +++ b/nginx/ngx_shared_dict.c   Mon Jun 12 18:33:05 2023 +0800
   @@ -45,18 +45,20 @@
   ngx_js_dict_add(ngx_js_dict_t *dict, njs_str_t *key, njs_str_t *value,
    ngx_msec_t expire)
   {
   +    size_t               n;
    uint32_t             hash;
    ngx_js_dict_node_t  *node;
   
    hash = ngx_crc32_long(key->start, key->length);

• node = ngx_slab_alloc_locked(dict->shpool, sizeof(ngx_js_dict_node_t));

• n = sizeof(ngx_js_dict_node_t) + key->length;

• node = ngx_slab_alloc_locked(dict->shpool, n); if (node == NULL) { if (dict->evict) { ngx_js_dict_evict(dict, 16);

• node = ngx_slab_alloc_locked(dict->shpool,

• sizeof(ngx_js_dict_node_t));

• node = ngx_slab_alloc_locked(dict->shpool, n); }

   if (node == NULL) {

  @@ -64,20 +66,7 @@ } }

• node->sn.str.data = ngx_slab_alloc_locked(dict->shpool, key->length);

• if (node->sn.str.data == NULL) {

• if (dict->evict) {

• ngx_js_dict_evict(dict, 16);

• node->sn.str.data = ngx_slab_alloc_locked(dict->shpool,

• sizeof(ngx_js_dict_node_t));

• }

• if (node->sn.str.data == NULL) {

• ngx_slab_free_locked(dict->shpool, node);

• return NGX_ERROR;

• }

• }

• node->sn.str.data = (u_char *) node + key->len;

  node->value.data = ngx_slab_alloc_locked(dict->shpool, value->length); if (node->value.data == NULL) { @@ -85,11 +74,10 @@ ngx_js_dict_evict(dict, 16);

       node->value.data = ngx_slab_alloc_locked(dict->shpool,

• sizeof(ngx_js_dict_node_t));

• value->length); }

   if (node->value.data == NULL) {

• ngx_slab_free_locked(dict->shpool, node->sn.str.data); ngx_slab_free_locked(dict->shpool, node); return NGX_ERROR; } @@ -160,7 +148,6 @@ ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);

  ngx_slab_free_locked(dict->shpool, node->value.data);

• ngx_slab_free_locked(dict->shpool, node->sn.str.data); ngx_slab_free_locked(dict->shpool, node); }

@@ -251,7 +238,6 @@ ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);

     ngx_slab_free_locked(dict->shpool, node->value.data);

• ngx_slab_free_locked(dict->shpool, node->sn.str.data); ngx_slab_free_locked(dict->shpool, node); } } @@ -288,7 +274,6 @@ ngx_rbtree_delete(&dict->sh->rbtree, (ngx_rbtree_node_t *) node);

   ngx_slab_free_locked(dict->shpool, node->value.data);

• ngx_slab_free_locked(dict->shpool, node->sn.str.data); ngx_slab_free_locked(dict->shpool, node); } }

[xeioex]

@hongzhidao

Thanks for reviewing the code, it was helpful.

What's the intention of the usage?

Yes, the original intention was to have a shortcut when there is only one shared dictionary. But after some thinking I decided to remove it, to simplify the code and not to confuse the users.

Handing the case of empty value.

Thanks, fixed the test case. Also reworked the deletion, so it is always explicit and empty strings are also allowed now.

Just an idea to simplify related allocation and free.

That does not work when we want to replace the values, we want to reallocate only the value part.

[hongzhidao]

@xeioex You are welcome.

That does not work when we want to replace the values, we want to reallocate only the value part.

You are right, but take a look again, I only combine the node structure with the key.

-    node = ngx_slab_alloc_locked(dict->shpool, sizeof(ngx_js_dict_node_t));
+    n = sizeof(ngx_js_dict_node_t) + key->length;

[xeioex]

@drsm, @hongzhidao

A topic for discussion

What ngx.shared.foo.set() should do when 1) memory is not enough

Should it return a boolean value signifying success/failure (as of now), or it should throw an exception?

What would be intuitive and convenient for users?

For the context, in the first patch the only reason of failure was non enough memory, and a boolean was sufficient. But I am working now on must_exist | should_not_exist flags. As a result there will be at least two different reasons for a falure of set().

[hongzhidao]

My thoughts are:

1. It would be useful to log error when it happens.
2. I found, for now, the only case to return failure for set() is the shared memory is not enough, but there might more cases like "too long length for key", or "empty key". So I prefer to throw an exception to indicate not enough memory happened.

BTW, do we need to handle the empty key for both set and delete?

[xeioex]

@hongzhidao

BTW, do we need to handle the empty key for both set and delete?

No, we do not. I reworked the part with deletion, so it is done explicitly.

[xeioex]

@hongzhidao here is the latest version

https://gist.github.com/xeioex/b61de2bba239cc84bf7c459966367ddf

[jirutka]

# Declares a shared dictionary of strings of size 1 Mb, that
# removes key-value after 60 seconds of inactivity.
js_shared_dict_zone zone=foo:1M timeout=60s;

I find timeout a bit confusing, I think that ttl would be more accurate. When I see timeout, I think about network timeout, operation timeout, …, an interval after which something is aborted as a failure. On the other hand, timeout in the meaning of TTL is already used in keyval_zone, so it’s probably better to be consistent with that.

What ngx.shared.foo.set() should do when memory is not enough Should it return a boolean value signifying success/failure (as of now), or it should throw an exception?

It should definitely throw an exception for the reasons already mentioned. Also, returning a boolean to indicate a failure is not a good practice in general (except in C…) – it often leads to silent/unhandled errors because devs forget or don’t care to handle it.

But I am working now on must_exist | should_not_exist flags.

Can you please elaborate on this?

[drsm]

@xeioex

I think we should throw on allocation failure. So one can distinguish a case of existent|nonexistent value from a low memory conditions.

BTW,

-    items(): number;
+   readonly size: number; // like in Map, Set

-    freeSpace(): number;
+   readonly freeSpace: number;

Also, it would be nice to have a has method, to reduce a boilerplate in a case of shared set.

[jirutka]

items(): number;
freeSpace(): number;

As @drm already said, readonly size: number and readonly freeSpace: number would be better.

> get(id: string): NgxSharedDictValue;

What will this do if an entry with the specified id does not exist? To be consistent with Object, Map, Set… it should return undefined, so the return type should be NgxSharedDictValue | undefined.

> /**
>  * Sets a value by key.
>  * @param id is a string key.
>  * @param value is a string or number value.
>  * @param flags is in optional number value with flags.
>  * 1 - the value must exist in the dictionary (the same as replace()).
>  * 2 - the value must not exist in the dictionary (the same as add()).
>  * 0 is the default value (no flags).
>  * @returns true if the value has been successfully set,
>  * and false otherwise.
>  */
> set(id: string, value:NgxSharedDictValue, flags?:number): boolean;

I’d prefer making set() consistent with Map.prototype.set():

• drop flags – they’re redundant, you have separate methods (add and replace) doing the same thing.
• return “this” instead of boolean to allow chaining

incr(id: string, delta: number, init?: number): number;

What will this method do if called on a dictionary of type string?

Can you also consider adding the following methods?

• has(id: string): boolean – returns a boolean indicating whether an element with the specified key exists or not
• keys(): IterableIterator<string> (or keys(): string[]) – returns an interator that contains the keys for each entry in the dict
• clear(): void – removes all entries from the dict

[drsm]

@xeioex @jirutka

How about:

readonly name: string
readonly capacity: number;
readonly freeSpace: number;
incr(id: string, delta: number, init: number): number;

// mimic JS Map
readony size: number;
set(id: string, value:NgxSharedDictValue): NgxSharedDict;
delete(id: string): boolean;
get(id: string): NgxSharedDictValue | undefined;
has(id: string): boolean;
keys(): string[];
clear(): void;

// SQL like setters
insert(id: string, value:NgxSharedDictValue): boolean;
update(id: string, value:NgxSharedDictValue): boolean;

[xeioex]

@jirutka

As @drm already said, readonly size: number and readonly freeSpace: number would be better

Originally, when making capacity as property and freeSpace() and items(), the distinction was that the first is a constant and the latter are variables. Also the methods signify for the user that accessing it is not cost-free. As for example with items() we in order to count the items block the whole tree.

[jirutka]

Originally, when making capacity as property and freeSpace() and items(), the distinction was that the first is a constant and the latter are variables.

This makes sense. However, I find items() confusing, I would expect it to return a list or iterator over the items in the dictionary. Naming it size() would be better.

[xeioex]

@jirutka, @drsm

On the other hand, timeout in the meaning of TTL is already used in keyval_zone, so it’s probably better to be consistent with that.

yes, that was my intention to be consistent with keyval.

But I am working now on must_exist | should_not_exist flags.

Can you please elaborate on this?

That was about flags for the set() method.

drop flags – they’re redundant, you have separate methods (add and replace) doing the same thing.

I was thinking about allowing a user to choose programmatically the behaviour of set(), but I agree it is probably redundant to add() and replace().

incr(id: string, delta: number, init?: number): number; What will this method do if called on a dictionary of type string?

It throws an exception now.

Can you also consider adding the following methods?

Yes, I plan to add them. I am thinking about

readonly name: string;
readonly capacity: number;
freeSpace(): number;  // method because it involves calculation
incr(id: string, delta: number, init: number): number;

size(): number;  // method because it involves counting
set(id: string, value:NgxSharedDictValue): NgxSharedDict;
delete(id: string): boolean;
get(id: string): NgxSharedDictValue | undefined;
has(id: string): boolean;
keys(max_count?:number): string[]; // optional limit for performance
clear(): void;

[hongzhidao]

delete(id: string): boolean;

What does this return value represent?

[hongzhidao]

@xeioex

@hongzhidao here is the latest version https://gist.github.com/xeioex/b61de2bba239cc84bf7c459966367ddf

I reworked it. But since you plan to simplify the APIs related to "set/add/replace", the patch is to show where could be reworked. https://gist.github.com/hongzhidao/1eb725273389c1982adc4af093079847

1. For the APIs ngx_js_dict_{name}(), dict is more readable than shm_zone as its first parameter.
2. Introduced something like ngx_js_dict_alloc/free() as a helper to simplify related allocation and release.
3. Tiny fixes.

[xeioex]

@hongzhidao

What does this return value represent?

false will be returned when a value to delete was absent.

[hongzhidao]

@hongzhidao

What does this return value represent?

false will be returned when a value to delete was absent.

I thought this was your intention, and it seems you missed something here.

    ngx_js_dict_delete(shm_zone->data, &key);  /* should do something on it */

    njs_value_boolean_set(retval, 1);

[xeioex]

@drsm, @jirutka, @hongzhidao

https://gist.github.com/xeioex/64e78d037bedb491bd1ebd3c48a5f78f

Here is the version the incorporates recent suggests.

1) clear(), has(), keys() were added 2) items() renamed to size() 3) add(), set(), replace() now throw exception if memory is not enough and return this 4) code improvements by @hongzhidao were merges with minor changes

[hongzhidao]

@xeioex Take a look. I have a patch to simplify related "expire" code later, but it's based on a question I showed below.

diff -r 09951503ac64 nginx/ngx_js_shared_dict.c
--- a/nginx/ngx_js_shared_dict.c        Wed Jun 14 23:00:10 2023 -0700
+++ b/nginx/ngx_js_shared_dict.c        Thu Jun 15 19:14:55 2023 +0800
@@ -573,8 +573,7 @@
     rbtree = &dict->sh->rbtree;

     if (rbtree->root == rbtree->sentinel) {
-        ngx_rwlock_unlock(&dict->sh->rwlock);
-        return NJS_OK;
+        goto done;
     }

     for (rn = ngx_rbtree_min(rbtree->root, rbtree->sentinel);
@@ -589,19 +588,27 @@

         value = njs_vm_array_push(vm, retval);
         if (value == NULL) {
-            return NJS_ERROR;
+            goto fail;
         }

         rc = njs_vm_value_string_set(vm, value, node->sn.str.data,
                                      node->sn.str.len);
         if (rc != NJS_OK) {
-            return NJS_ERROR;
+            goto fail;
         }
     }

+done:
+
     ngx_rwlock_unlock(&dict->sh->rwlock);

     return NJS_OK;
+
+fail:
+
+    ngx_rwlock_unlock(&dict->sh->rwlock);
+
+    return NJS_ERROR;
 }

@@ -1166,9 +1173,11 @@

     if (node == NULL) {
         if (flags & NGX_JS_DICT_FLAG_MUST_EXIST) {
-            ngx_rwlock_unlock(&dict->sh->rwlock);
-            njs_vm_error(vm, "value to replace not found");
-            return NJS_ERROR;
+            if (!dict->timeout || now < node->expire.key) {
+                ngx_rwlock_unlock(&dict->sh->rwlock);
+                njs_vm_error(vm, "value to replace not found");
+                return NJS_ERROR;
+            }
         }

         ngx_js_dict_expire(dict);

Since the has() function is introduced, do you think we still need add() and replace()? I'm thinking about whether we could remove the internal flags of "must_exist" and "must_not_exist". If yes, we can simplify related code. @xeioex @jirutka @drsm what do you think? Does it make sense to get rid of add() and replace()?

[xeioex]

@hongzhidao

I think it is important to have add() and replace() independently from has() for convenience and performance in some cases. Also lua module has these methods, and having the same primitives would help users in migration to njs if they want to.

[hongzhidao]

@xeioex Take a look. https://gist.github.com/hongzhidao/8c3220b1ca2e1d6bc1fe789d1674f59a

Btw, A few places about the commit log:

1. s/The directives allows to declared/The directives allows to declare/

2. function get(r) {
           r.return(200, ngx.shared.foo.get(r.args.key));
   } /* missed } */

3. What about adding blank lines like this:

       # Declares a shared dictionary of strings of size 1 Mb, that
       # removes key-value after 60 seconds of inactivity.
       js_shared_dict_zone zone=foo:1M timeout=60s;
   
       # Declares a shared dictionary of strings of size 512Kb, that
       # forsibly remove oldest key-value pairs when memory is not enough.
       js_shared_dict_zone zone=bar:512K timeout=30s evict;
   
       # Declares a permanent number shaed dictionary of size 32Kb.
       js_shared_dict_zone zone=num:32k type=number;

[xeioex]

@hongzhidao

ngx_js_dict_node_delete()

I think, this method is to ad-hoc, so I prefer to leave ngx_js_dict_node_free() as is, even it result in several extra lines of code.

Otherwise, I will try to apply your suggestions if they fit enough.

[hongzhidao]

I prefer to leave ngx_js_dict_node_free() as is

Sure, I'm fine with it.

Btw, What about reordering the internal APIs? I have no clear idea about it, but I think it's worth doing it so that developers can understand the source code easier. And yours is a good reference.

readonly name: string;
readonly capacity: number;
freeSpace(): number;  // method because it involves calculation
size(): number;  // method because it involves counting

get(id: string): NgxSharedDictValue | undefined;
has(id: string): boolean;
keys(max_count?:number): string[]; // optional limit for performance

incr(id: string, delta: number, init: number): number;
set(id: string, value:NgxSharedDictValue): NgxSharedDict;

delete(id: string): boolean;
clear(): void;

For example, putting the ngx_js_dict_set/add/update() together.

[drsm]

@xeioex

add(), set(), replace() now throw exception if memory is not enough and return this

Do we need chaining in add|replace ? While lua module also does this, an exception when an absent key has been replaced is little bit confusing. Also, as for now, one can't catch it by type. Boolean result looks better to me.

[xeioex]

@hongzhidao The current order is alphabetical for njs method, may be it is not ideal, but at least it is a definite order. Also the internal routines are moved to the bottom of the file. But I can group ngx_js_dict_set/add/update() more tightly.

[xeioex]

@drsm

Do we need chaining in add|replace ? While lua module also does this, an [exception]> (https://gist.github.com/xeioex/64e78d037bedb491bd1ebd3c48a5f78f#file-shared_dict3-L1473) when an absent key has >been replaced is little bit confusing. Also, as for now, one can't catch it by type. Boolean result looks better to me.

I did the chaining for set() to move the interface more closely to Map as you suggested. But I still want to see the usefulness of such chaining construction for set() method in practice. I do love chaining in Array methods like map(), filter() where the chaining is clearly useful.

[xeioex]

@hongzhidao

shared_dict v4 https://gist.github.com/xeioex/6b7c18c969dfa0189e4e2d9ab6d4a4af

applied improvement from here.

@hongzhidao, @jirutka

Do you have any opinion on drsm comment what should be the retval of add() and replace().

[hongzhidao]

@xeioex

shared_dict v4 https://gist.github.com/xeioex/6b7c18c969dfa0189e4e2d9ab6d4a4af

Great job, no issue found. And here's a small rework. Others look good. https://gist.github.com/hongzhidao/48a5be0e82a5c1a01208d1012f83ba3c

[drsm]

@xeioex

But I still want to see the usefulness of such chaining construction for set() method in practice.

I don't have any arguments for chaining in set() other than POLA.

But, for add() or replace() it will made error control painful.

dumb PoC:

try {
    sid = null;

    // just try to do the same with exceptions
    if (!njx.shared.user.add(uid, Date.now())) {
        return processSuspiciousLogin(r, uid);
    }

    sid = generateSid();

    njx.shared.session.set(sid, uid);

    // ...
    return true;

} catch (e) {
    if (e instanceof MemoryError) {
        // it would be nice to have SharedMemoryError, 'cause MemoryError may came from a different source
        njx.shared.user.remove(uid);
        r.log(`unable to login ${uid}, consider increasing ${sid ? 'session' : 'user'} shared memory zone`);

    } else {
        // something unexpected
    }

    return false;
}

[xeioex]

@drsm

But, for add() or replace() it will made error control painful.

Thanks, that was a good example. I will update the retval for add() and replace().

[hongzhidao]

@xeioex

Great job, no issue found. And here's a small rework. Others look good. https://gist.github.com/hongzhidao/48a5be0e82a5c1a01208d1012f83ba3c

Introduced a mistake here. https://gist.github.com/hongzhidao/48a5be0e82a5c1a01208d1012f83ba3c#file-shared_dict4-patch-L60

Updated with:

+    njs_value_boolean_set(retval, node != NULL);
     ngx_rwlock_unlock(&dict->sh->rwlock);
-    njs_value_boolean_set(retval, has);

[xeioex]

@hongzhidao

Introduced a mistake here

Yes, I already fixed it.

[jirutka]

Do you have any opinion on drsm comment what should be the retval of add() and replace().

I agree with @drsm. Since add should set a value by key only if the key does not exist yet, it should return true when the key didn’t exist and so the value was set, false otherwise. replace should replace a value by key only if the key exists, so it should return true when the key exists and so the value was replaced, false otherwise. In all cases, out-of-memory should be indicated by an exception.

The question is, what exception type? It’ll be better to “subclass” Error (or some other error type), so it can be disguised from other error cases. @drsm suggested SharedMemoryError, I would choose something more specific, e.g. OutOfSharedMemoryError?

[jirutka]

I also have an update of the types, but I cannot finish it now, I’ll add it later today.

[jirutka]

I rewrote the type declaration for NgxSharedDict to:

• use generic for the value type so users can (optionally) assert a particular shared dictionary to its value type (const mydict = shared.mydict as NgxSharedDict<number>),
• forbid incr() method in NgxSharedDict<string>,
• rewrite the comments to be more informative (especially about errors) and consistent with existing types,
• highlight method arguments in the comment texts,
• declare the suggested OutOfSharedMemoryError type.

/**
 * This Error object is thrown when adding an item to a shared dictionary
 * that does not have enough free space.
 * @since 0.8.0
 */
declare class OutOfSharedMemoryError extends Error {}

/**
 * Interface of a dictionary shared among the working processes.
 * It can store either `string` or `number` values which is specified when
 * declaring the zone.
 * 
 * @type {V} The type of stored values.
 * @since 0.8.0
 */
interface NgxSharedDict<V extends string | number = string | number> {
    /**
     * The capacity of this shared dictionary in bytes.
     */
    readonly capacity: number;
    /**
     * The name of this shared dictionary.
     */
    readonly name: string;

    /**
     * Sets the `value` for the specified `key` in the dictionary only if the
     * `key` does not exist yet.
     * 
     * @param key The key of the item to add.
     * @param value The value of the item to add.
     * @returns `true` if the value has been added successfully, `false`
     *   if the `key` already exists in this dictionary.
     * @throws {OutOfSharedMemoryError} if there's not enough free space in this
     *   dictionary.
     * @throws {TypeError} if the `value` is of a different type than expected
     *   by this dictionary.
     */
    add(key: string, value: V): boolean;
    /**
     * Removes all items from this dictionary.
     */
    clear(): void;
    /**
     * Removes the item associated with the specified `key` from the dictionary.
     * 
     * @param key The key of the item to remove.
     * @returns `true` if the item in the dictionary existed and has been
     *   removed, `false` otherwise.
     */
    delete(key: string): boolean;
    /**
     * Increments the value associated with the `key` by the given `delta`.
     * If the `key` doesn't exist, the item will be initialized to `init`.
     * 
     * **Important:** This method can be used only if the dictionary was
     * declared with `type=number`!
     * 
     * @param key is a string key.
     * @param delta The number to increment/decrement the value by.
     * @param init The number to initialize the item with if it didn't exist
     *   (default is `0`).
     * @returns The new value.
     * @throws {OutOfSharedMemoryError} if there's not enough free space in this
     *   dictionary.
     * @throws {TypeError} if this dictionary does not expect numbers.
     */
    incr: V extends number
      ? (key: string, delta: V, init?: number) => number
      : never;
    /**
     * @returns The free page size in bytes.
     *   Note that even if the free page is zero the dictionary may still accept
     *   new values if there is enough space in the occupied pages.
     */
    freeSpace(): number;
    /**
     * @param key The key of the item to retrieve.
     * @returns The value associated with the `key`, or `undefined` if there
     *   is none.
     */
    get(key: string): V | undefined;
    /**
     * @param key The key to search for.
     * @returns `true` if an item with the specified `key` exists, `false`
     *   otherwise.
     */
    has(key: string): boolean;
    /**
     * @param maxCount The maximum number of keys to retrieve (default is 1024).
     * @returns An array of the dictionary keys.
     */
    keys(maxCount?: number): string[];
    /**
     * Sets the `value` for the specified `key` in the dictionary only if the
     * `key` already exists.
     * 
     * @param key The key of the item to replace.
     * @param value The new value of the item.
     * @returns `true` if the value has been replaced successfully, `false`
     *   if the key doesn't exist in this dictionary.
     * @throws {OutOfSharedMemoryError} if there's not enough free space in this
     *   dictionary.
     * @throws {TypeError} if the `value` is of a different type than expected
     *   by this dictionary.
     */
    replace(key: string, value: V): boolean;
    /**
     * Sets the `value` for the specified `key` in the dictionary.
     *  
     * @param key The key of the item to set.
     * @param value The value of the item to set.
     * @returns This dictionary (for method chaining).
     * @throws {OutOfSharedMemoryError} if there's not enough free space in this
     *   dictionary.
     * @throws {TypeError} if the `value` is of a different type than expected
     *   by this dictionary.
     */
    set(key: string, value: V): this;
    /**
     * @returns The number of items in this shared dictionary.
     */
    size(): number;
}

interface NgxGlobalShared {
    /**
     * Shared dictionaries.
     * @since 0.8.0
     */
    readonly [name: string]: NgxSharedDict;
}

[jirutka]

if (dict->type == NGX_JS_DICT_TYPE_STRING) {
    if (!njs_value_is_string(value)) {
        njs_vm_error(vm, "string value is expected");
        return NJS_ERROR;
    }
} else {
    if (!njs_value_is_number(value)) {
        njs_vm_error(vm, "number value is expected");
        return NJS_ERROR;
    }
}

This throws Error, right? It would be better to throw TypeError instead.

[jirutka]

It would also be useful to have a method that will atomically return and delete a value specified by the key. We can either change the delete() method or add another method, e.g. pop() – if we wanna stick to the Map interface and/or if returning the deleted object have a measurable performance impact.

[xeioex]

Hi @jirutka and @drsm,

Thank you for good suggestions. I plan to implement them.

Currently I am working on allowing external modules (like the shared dict code in the patch) to create its own exception types cheaply.

[xeioex]

@jirutka, @hongzhidao, @drsm

Here is the final shared dict patch: https://gist.github.com/xeioex/1429e987b0f585ef9eb599b49fcf5a5c

It includes 1) SharedMemoryError 2) pop() method 3) TypeError exceptions for unexpected values 4) TypeScript updates from @jirutka

[jirutka]

- pop(key: string): boolean;
+ pop(key: string): V | undefined;

[hongzhidao]

@xeioex

Here is the final shared dict patch: https://gist.github.com/xeioex/1429e987b0f585ef9eb599b49fcf5a5c

Looks good to me.

[jirutka]

- * @type {V} The type of stored values.
+ * @template V The type of stored values.

[xeioex]

@drsm, @jirutka, @hongzhidao

Thanks guys, committed. I appreciate your contribution.