Skip to content

Commit 86e1201

Browse files
adiholdenromange
authored andcommitted
server: disable single shard tx optimization on scheduling (#4647)
Signed-off-by: adi_holden <adi@dragonflydb.io>
1 parent 9e52438 commit 86e1201

File tree

3 files changed

+16
-3
lines changed

3 files changed

+16
-3
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ jobs:
3232
path: ~/.cache/pre-commit
3333
key: pre-commit|${{ env.pythonLocation }}|${{ hashFiles('.pre-commit-config.yaml') }}
3434
- name: Run pre-commit checks
35-
run:
35+
run: |
3636
source venv/bin/activate
3737
pre-commit run --show-diff-on-failure --color=always --from-ref HEAD^ --to-ref HEAD
3838
shell: bash

src/server/test_utils.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ void BaseFamilyTest::ResetService() {
265265
LOG(ERROR) << "Deadlock detected!!!!";
266266
absl::SetFlag(&FLAGS_alsologtostderr, true);
267267
fb2::Mutex m;
268-
shard_set->pool()->AwaitFiberOnAll([&m](unsigned index, ProactorBase* base) {
268+
shard_set->pool()->AwaitFiberOnAll([&m, this](unsigned index, ProactorBase* base) {
269269
ThisFiber::SetName("Watchdog");
270270
std::unique_lock lk(m);
271271
LOG(ERROR) << "Proactor " << index << ":\n";
@@ -293,6 +293,14 @@ void BaseFamilyTest::ResetService() {
293293
->trans_locks) {
294294
LOG(ERROR) << "Key " << k_v.first << " " << k_v.second;
295295
}
296+
297+
LOG(ERROR) << "Transaction for shard " << es->shard_id();
298+
for (auto& conn : connections_) {
299+
auto* context = conn.second->cmd_cntx();
300+
if (context->transaction && context->transaction->IsActive(es->shard_id())) {
301+
LOG(ERROR) << context->transaction->DebugId(es->shard_id());
302+
}
303+
}
296304
}
297305
});
298306
}

src/server/transaction.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,7 @@ string Transaction::DebugId(std::optional<ShardId> sid) const {
556556
absl::StrAppend(&res, ":", multi_->cmd_seq_num);
557557
}
558558
absl::StrAppend(&res, " {id=", trans_id(this));
559+
absl::StrAppend(&res, " {cb_ptr=", absl::StrFormat("%p", static_cast<const void*>(cb_ptr_)));
559560
if (sid) {
560561
absl::StrAppend(&res, ",mask[", *sid, "]=", int(shard_data_[SidToId(*sid)].local_mask),
561562
",is_armed=", DEBUG_IsArmedInShard(*sid),
@@ -756,7 +757,11 @@ void Transaction::ScheduleInternal() {
756757

757758
ScheduleContext schedule_ctx{this, optimistic_exec};
758759

759-
if (unique_shard_cnt_ == 1) {
760+
// TODO: this optimization is disabled due to a issue #4648 revealing this code can
761+
// lead to transaction not being scheduled.
762+
// To reproduce the bug remove the false in the condition and run
763+
// ./list_family_test --gtest_filter=*AwakeMulti on alpine machine
764+
if (false && unique_shard_cnt_ == 1) {
760765
// Single shard optimization. Note: we could apply the same optimization
761766
// to multi-shard transactions as well by creating a vector of ScheduleContext.
762767
schedule_queues[unique_shard_id_].queue.Push(&schedule_ctx);

0 commit comments

Comments
 (0)