fix: Don't remove non-concluding tx from queue on ooo runs #1427
Conversation
Signed-off-by: Vladislav Oleshko <[email protected]>
src/server/engine_shard_set.cc
Outdated
| bool keep = trans->RunInShard(this, true); | ||
| DLOG_IF(INFO, !dbg_id.empty()) << "Eager run " << sid << ", " << dbg_id << ", keep " << keep; |
There was a problem hiding this comment.
Maybe I should add there a DCHECK_EQ(trans->GetTxqPos(shard) != TxQueue::kEnd, keep)? Requires adding GetTxqPos
Signed-off-by: Vladislav Oleshko <[email protected]>
src/server/engine_shard_set.cc
Outdated
| @@ -390,7 +390,7 @@ void EngineShard::PollExecution(const char* context, Transaction* trans) { | |||
| } | |||
| ++stats_.ooo_runs; | |||
|
|
|||
| bool keep = trans->RunInShard(this); | |||
| bool keep = trans->RunInShard(this, true); | |||
There was a problem hiding this comment.
should_run is also set for SUSPENDED_Q.
Why not be more precise and pass trans_mask & Transaction::OUT_OF_ORDER ?
There was a problem hiding this comment.
Moreover, to avoid the situation where both bits are set, I suggest to add
sd.local_mask |= SUSPENDED_Q;
sd.local_mask &= ~OUT_OF_ORDER; /
in WatchInShard
There was a problem hiding this comment.
True... I have actually a parallel PR where I remove the unwatch hop and make all wakeups single hop 🙄
But generally running blocked ooo should be safe because they have they run directly in PollExecution and should have only one hop after wakeups 🤔
Signed-off-by: Vladislav Oleshko <[email protected]>
| @@ -243,6 +245,10 @@ class Transaction { | |||
| return shard_data_[SidToId(sid)].local_mask; | |||
| } | |||
|
|
|||
| uint32_t GetLocalTxqPos(ShardId sid) const { | |||
There was a problem hiding this comment.
you do not need it. There is PqPos for that. Remove either of them.
Fixes the long hunted blpop bug