Skip to content

Threaded commands implementation #7372

@yossigo

Description

@yossigo

Hi @antirez, I have looked at the threaded commands implementation (threaded-core-commands up to a5541e8) and opening this issue to discuss that.

I think this is a great use case and a validation of past concepts like the blocking mechanism that were part of the modules (and btw may need to be refactored out of the module subsystem just for code clarity).

My main feedback is this: the locking mechanism needs to be invoked explicitly by every command that uses it. This not only requires a lot of code change that make it less readable, but may also be unnecessary given that key access is known in advance so it could be a core function. The only piece of information missing to do that is the ability to specify read-only/read-write keys at the command table level. I'm sure that would be easier and safer to do than modify lots of commands.

Other concerns:

  • Performance: I think spawning a thread per command is not efficient, and a thread pool should be used.
  • Performance: there is a fast path check that guarantees no penalty when no keys are locked. However in real world scenarios there will always be locked keys - and I have some concerns about the latency impact of clientShouldWaitOnLockedKeys().
  • Safety: the lockKey() practice of silently ignoring repeated locks by the same client is extremely dangerous and should be avoided (unless the lock is recursive... but really, let's not!).

Other thoughts:

  • Moving forward to read/write locking is not trivial, I think it will take some design work that hopefully will also consider some of the points above.
  • How shall Lua blocking be handled? Blocking only EVAL itself based on declared keys may lead to locking violation if scripts are not well behaved, which is a regression. It may be necessary to strictly enforce key access, or make it possible to block Lua clients (which will complicate things a lot).
  • Same question but for modules, which have multiple different flows: RM_Call() on main thread, RM_Call() on a thread safe context, direct API key access.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    Backlog

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions