feat(transaction): Single hop blocking, callback flags #2393
Conversation
Signed-off-by: Vladislav <[email protected]>
Signed-off-by: Vladislav Oleshko <[email protected]>
src/server/transaction.cc
Outdated
| if (result == OpStatus::OUT_OF_MEMORY) { | ||
| local_result_ = result; // TODO: What??? |
There was a problem hiding this comment.
How is this safe?... (more than 1 shard)
Signed-off-by: Vladislav Oleshko <[email protected]>
src/server/transaction.cc
Outdated
|
|
||
| // Handle result flags to alter behaviour. | ||
| if (result.flags & RunnableResult::AVOID_CONCLUDING) { | ||
| CHECK_EQ(unique_shard_cnt_, 1u); // multi shard must know it ahead, so why do those tricks? |
There was a problem hiding this comment.
I do not understand the comment. what tricks? what multi-shard must know?
There was a problem hiding this comment.
Changed the comment, multi shard must conclude either all or none, because the cbs don't coordinate it must know it ahead -> no point in using this flag
|
looks simple but scary at the same time. |
|
@adiholden please go over as well. Also, left you a question there too. |
| shard->PollExecution("schedule_unique", nullptr); | ||
| } | ||
|
|
||
| return quick_run; |
There was a problem hiding this comment.
what if quick_run = true but esult.flags is RunnableResult::AVOID_CONCLUDING
should we return false in this case?
There was a problem hiding this comment.
True, because the callback did run and won't run again until requested. This is why quick_run and lock_keys are now different variables. Also I updated the comment above, false means the callback will be run via tx queue (with the flag it won't run again until the next Execute)
Signed-off-by: Vladislav <[email protected]> Signed-off-by: Vladislav Oleshko <[email protected]>
We have lots of existing tests that cover both paths, but the coverage density is less now |
Optimize blocking command hot path to be ONE hop instead of three in single shard case.
If BLPOP runs only on a single shard and detects that the keys exists, we can actually perform the action in the same hop. If the key doesn't exist, we have to tell the transaction that we changed our mind in the last very second and like to continue 🙂
TODO: check streams?(Doesn't use container utils)TODO: bench?(Done by Kostas)TODO: check journal?(Done)