Fix timestamp overflow in lock cleanup causing permanent lock leaks#314
Fix timestamp overflow in lock cleanup causing permanent lock leaks#314bootjp merged 7 commits intofeature/multi-shard-transactionfrom
Conversation
Replace cleanupTS with abortTSFrom to handle overflow when startTS==^uint64(0). Add explicit overflow checks to prevent invalid abort timestamps that violate the FSM requirement that resolveTS > startTS, which could leave locks permanently unresolved. Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Pass commitTS from primaryTxnStatus to abortTSFrom for semantic clarity and consistency with usage elsewhere in the codebase. Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Explain why startTS is passed twice to abortTSFrom: no commitTS is available when aborting an expired lock with no commit record. Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Use 0 instead of startTS as the second parameter to abortTSFrom to explicitly indicate that no commitTS is available in this context. Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
|
@copilot golangci落ちています |
|
@copilot Uint64なので通常オーバーフローが起きない想定だが、バグで起きることを考慮したロジックであることをコメントに書いてください |
Remove extra blank lines left after deleting cleanupTS function to fix golangci-lint gci formatting check. Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Clarify that while uint64 overflow is not expected in normal operation, the overflow checks are defensive programming to handle the edge case where startTS==^uint64(0) or a bug causes overflow, preventing violation of the FSM invariant resolveTS > startTS. Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Fixed in commit a3477b6. Added defensive programming comments at all three overflow check locations, explaining that while uint64 overflow is not expected in normal operation, these checks handle the edge case where startTS==^uint64(0) or a bug causes overflow, preventing violation of the FSM invariant. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where timestamp overflow during lock cleanup could cause permanent lock leaks. The fix replaces the cleanupTS function with abortTSFrom, which properly handles edge cases where incrementing timestamps would overflow (e.g., when startTS == ^uint64(0)). The new implementation returns 0 when no valid abort timestamp can be computed, and all three call sites add defensive checks to prevent violating the FSM invariant that requires resolveTS > startTS.
Changes:
- Replaced
cleanupTSwithabortTSFromfor computing abort timestamps with overflow safety - Added overflow checks at three critical lock resolution points to prevent FSM invariant violations
- Removed the obsolete
cleanupTSfunction that lacked overflow handling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cleanupTSwithabortTSFromto handle timestamp overflowcleanupTSis called💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.