Revert "Range read block on rw conflict (#283)"

[liunyl]

This reverts commit 7c0a426.
Causes checkpoint unable to proceed

Here are some reminders before you submit the pull request

• Add tests for the change
• Document changes
• Reference the link of issue using fixes eloqdb/tx_service#issue_id
• Reference the link of RFC if exists
• Pass ./mtr --suite=mono_main,mono_multi,mono_basic

Summary by CodeRabbit

• Refactor
  • Improved transaction conflict resolution for concurrent read/write operations by optimizing retry behavior and reducing suspension periods.
  • Streamlined internal synchronization mechanisms to decrease conflict detection overhead.
  • Enhanced transaction recovery logic when operations encounter lock conflicts.
  • Simplified transaction processing paths by removing legacy diagnostic code.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

The changes modify lock acquisition and deadlock detection behavior in the transaction service. When a read lock is blocked by a write lock, the system now aborts the transaction and releases held locks instead of blocking or performing deadlock checks. Additionally, the deadlock detection mechanism is refactored to use mutex-protected booleans instead of atomic operations, and transaction operations implement conditional abort-and-retry logic based on existing acquired reads.

Changes

Cohort / File(s)	Summary
Deadlock detection refactoring tx_service/include/dead_lock_check.h, tx_service/src/dead_lock_check.cpp	Changed requested_check_ from std::atomic<bool> to mutex-protected bool, replacing atomic load/store operations with direct boolean access. Updated wait-condition logic and flag reset to use non-atomic semantics.
ReadCc lock-block handling tx_service/include/cc/range_cc_map.h	Removed deadlock_check dependency in ReadCc path. When lock acquisition is blocked, now releases the bucket read lock, aborts the bucket key lock queue with ACQUIRE_KEY_LOCK_FAILED_FOR_RW_CONFLICT, and returns true instead of blocking. Replaced assertion allowing resumed requests with assertion forbidding them.
Transaction operation abort-and-retry logic tx_service/src/tx_operation.cpp	Added conditional logic in ReadLocalOperation::Forward and LockWriteRangeBucketsOp::Forward to inspect metadata read sets when ACQUIRE_KEY_LOCK_FAILED_FOR_RW_CONFLICT occurs—abort if non-metadata reads exist, otherwise retry. Removed fault-injection code in SplitFlushRangeOp commit path and adjusted logging consistency.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

• Lock release semantics: Verify that lock release always precedes abort in ReadCc path to prevent resource leaks or deadlocks.
• Conditional abort-retry logic in tx_operation.cpp: Review the metadata read set inspection carefully—ensure the abort decision is correct and retry conditions don't reintroduce deadlocks.
• Mutex-protected boolean correctness: Confirm all accesses to requested_check_ in dead_lock_check are properly synchronized and that removing atomics doesn't introduce races.
• Fault-injection removal impact: Verify that removing the SplitFlushRangeOp fault-injection block doesn't mask real deadlock scenarios or change test coverage expectations.

Possibly related PRs

• disable filter pushdown for scan command #238: Removes DeadLockCheck::RequestCheck usage in ScanOpenOperation::Forward, directly aligning with the deadlock_check refactoring in this PR.
• Range read block on rw conflict #283: Modifies the same ReadCc and deadlock_check paths but implements opposite behavior (enables blocking/deadlock detection instead of aborting), providing context for the architectural shift.

Poem

🐰 Locks once blocked and deadlocks were checked,
Till transactions sadly wrecked,
Now we abort and quickly release,
Retry with metadata peace!
No atoms needed, just mutex care,
Deadlock avoidance in the air! ✨

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch revert_bug

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0c6d867 and 8aaaab9.

📒 Files selected for processing (4)

• tx_service/include/cc/range_cc_map.h (2 hunks)
• tx_service/include/dead_lock_check.h (2 hunks)
• tx_service/src/dead_lock_check.cpp (3 hunks)
• tx_service/src/tx_operation.cpp (3 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Ubuntu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}