New atomic functions(cas, cog) for shm

[Miniwoffer]

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

See: #1954

[xiaocang]

Hi, @Miniwoffer I was wondering what the cas and cog abbreviations refer to? seems not very common

[Miniwoffer]

cas(compare and swap)
cog(compare or get)

cas refers to the atomic operation(https://en.wikipedia.org/wiki/Compare-and-swap), and is also used by memcached amogst others.
cog is not a common abbreviations, just something derived from cas.

[xiaocang]

LGTM.

@zhuizhuhaomeng @doujiang24 could you help with this PR ?

[doujiang24]

I'm fine with the cas API. We'd better wait for @agentzh 's comment for the cos API.

[agentzh]

What does the "only returns if" part mean? If the if condition is not met, this method will never return? It cannot be. Maybe you mean "will only return the value and the flags" here? And you should also be explicit about what values it returns otherwise (like two nil values?)

The cog name is a bit confusing. How about get_if_not_eq?

[Miniwoffer]

Yeah the "only returns if" part is not true.
Maybe something like:

Similar to the [get](#ngxshareddictget) method, but if ``old_value`` and
``old_flags`` match value and flag in dictionary ngx.shared.DICT, 
returns ``nil, true``.

I'm fine with get_if_not_eq

[agentzh]

This semantics does not feel right to me. When old value is nil, then it means the expected value in the shdict should also be nil. It should not get ignored? So it always return nil, nilin this case?

[Miniwoffer]

The function first lookups the key, and if cant find it return nil, nil.
And after that apply comparison logic.

The reason for the "don't compare nil" is because of old_flags, and i simply tried to have old_value and old_flags act as similar as possible.

For old_flags i added a distinction between 0 and nil simply because it adds more functionality compared to having nil == 0.

Its also the fact that flags cannot exist if value is nil. Does that make flags nil or 0 maybe both maybe neither?

Having different logic where only old_flags is not compared when nil, might be best.

[bjne]

When old value is nil, it should be ignored when comparing (so value could be anything), when old flag is nil, flag is ignored.
This way the api user could have either old_value or old_flag (or both) and whichever is not nil will be compared to the value in shm

[xiaocang]

Hi @Miniwoffer, may I ask if there is a recent update commit in this PR?

[Miniwoffer]

Hi @Miniwoffer, may I ask if there is a recent update commit in this PR?

I was waiting for a comment on my answer to @agentzh s question. For some reason my comments are hidden

Il just commit my suggestions

[Miniwoffer]

@agentzh

In get_if_not_eq, nil for old_value or old_flag is equal to any value.
Why?
It adds more functionality.

In the case of get_if_not_eq(key, nil, flag) if we where to interpret nil
as literal nil it would simply just be a get(key).
But if we interpret nil as any it will act as a get_if_flag_not_eq(key, flag).

Same goes for get_if_not_eq(key, value, nil) this would be the same as
get_if_not_eq(key, value, 0) since 0==nil in shm.
But if we interpret nil as any it would act as a get_if_value_not_eq(key, value)

We could off course add

get_if_value_not_eq(key, value)
get_if_flag_not_eq(key, flag)

and have the same logic, if the API is too unintelligible.

I could also try and rewrite the documentation, if wording similar to nil==any
would help.

[mergify]

This pull request is now in conflict :(

[LoganDark]

I would strongly prefer compare_swap over cas, and for the other, compare_get, fetch or something. get_if_not_eq is such a horrible sore to type, I really hope a name like that never makes it into the main release.

[Miniwoffer]

@LoganDark
I agree that compare_swap and compare_get are cleaner.
The only disadvantage is that it's less self-documenting.

But then again maybe that can be an advantage, forcing the user not to make assumptions about its functionality.

@xiaocang
Also, this pull request has been idle for a while, and I'm unsure of what to do; is there anything I can do to get the process going?

[LoganDark]

it's less self-documenting

The best thing that can be done for documentation is to ensure the README is complete and up-to-date; this allows the creation of typings:

Since the README is already incredibly detailed and useful, I feel like compare_swap is more than good enough. But keep in mind that's just a suggestion. Even I think compare_get is a little weird, and possibly misleading...

But then again maybe that can be an advantage, forcing the user not to make assumptions about its functionality.

I would not consider this an advantage or a disadvantage. Warnings about implementation details belong in the documentation.