Threading in CCF

[ashamis]

No description provided.

[ashamis]

This was not meant to be closed, re-opening

[douglasism]

@ashamis Hi Alex, does this ticket and the linked issues represent all known multi-thread concurrency problems in CCF? That is, if I find a potential concurrency bug in a module not listed should I submit bugs?

[ashamis]

Hi @douglasism, this is still work in progress so I am fixing bugs as a I find them. If you find any concurrency bugs I would really appreciate any bug reports or PRs.

[douglasism]

@ashamis I've been reviewing KV. Cogent concurrency in KV is going to be challenging. Even once the current approach is completed (#855, and another one I'm working on), I'm not sure what statements one will be able to make about its soundness. More broadly, what are the multi-threading design goals in the enclave?

Also, have you looked into how the spinlocks get compiled? Do they actually test before waiting? IOW, are they no-ops if there is only one thread (or no contention) in the enclave?

[douglasism]

@ashamis (Alex, please take no offense from my suggestions. You probably know all this, but I feel amiss if I don't speak up when I see a problem.)

For KV, as I noted in CCF chat, there may be deadlock risk, given that there are multiple locks with unclear ordering.

Also, consider ways to get rid of this pattern:

    void rollback(Version v) override
    {
      // This is called to roll the store back to the state it was in
      // at the specified version.
      // No transactions can be prepared or committed during rollback.
      std::lock_guard<SpinLock> mguard(maps_lock);
...
      for (auto& map : maps)
        map.second->lock();

      for (auto& map : maps)
        map.second->rollback(v);

      for (auto& map : maps)
        map.second->unlock();
...

Would it be possible to push the map lock into the map::rollback() func, making it RAII? That way, if an exception is thrown, the locks will get released automatically, and it will clean up this code.

[ashamis]

@ashamis I've been reviewing KV. Cogent concurrency in KV is going to be challenging. Even once the current approach is completed (#855, and another one I'm working on), I'm not sure what statements one will be able to make about its soundness. More broadly, what are the multi-threading design goals in the enclave?

Also, have you looked into how the spinlocks get compiled? Do they actually test before waiting? IOW, are they no-ops if there is only one thread (or no contention) in the enclave?

@douglasism - The goal is to improve the throughput and reduce the latency of commands sent to CCF. Additionally, we want to isolate critical operations (such as Raft keep-alive messages) from command execution.

To generalize a bit the spinlocks we use wait on a variable that is updated with a CAS operation. At this time there is no special code in the single threaded case to reduce/avoid this cost. We may do something about this in the future but it will be prioritized against other performance issues.

[ashamis]

The bulk of this work is complete! If there are any other that pop-up they can be tracked individually.