Fix ComputeSublogAccessVector to use keyEntries#1939
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes sharded-AOF transaction sublog routing by computing the access bitmap from the transaction’s locked keys (available in both standalone and cluster modes), and adds coverage to ensure standalone implicit transactions (e.g., RENAME) recover correctly when AOF is sharded. It also updates standalone test infrastructure to allow BfTreeInterop.test to reuse Garnet.test internals and avoid port collisions.
Changes:
- Update
TransactionManager.ComputeSublogAccessVectorto usekeyEntries-backed key hashes for sublog routing (instead of cluster-only parse state). - Extend
TxnKeyEntrieswithCountandGetKeyHash(int)to support efficient routing without re-hashing. - Add a standalone recovery test for sharded AOF transactions and adjust test project wiring/port assignment for
BfTreeInterop.test.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/standalone/Garnet.test/TestUtils.cs | Adds a dedicated base port assignment for BfTreeInterop.test. |
| test/standalone/Garnet.test/Garnet.test.csproj | Grants internals access to BfTreeInterop.test (signed friend). |
| test/standalone/Garnet.test/AofShardedTxnRecoveryTests.cs | New test validating sharded-AOF implicit txn (RENAME) survives recovery in standalone mode. |
| test/standalone/BfTreeInterop.test/TestProjectSetup.cs | New setup fixture to set the test port for the BfTreeInterop.test project. |
| test/standalone/BfTreeInterop.test/BfTreeInterop.test.csproj | References Garnet.test to reuse shared test utilities. |
| libs/server/Transaction/TxnKeyEntry.cs | Adds TxnKeyEntries.Count and GetKeyHash(int) for key-hash access. |
| libs/server/Transaction/TransactionManager.cs | Switches sublog access-vector computation to use keyEntries hashes for correct routing in standalone sharded AOF. |
vazois
approved these changes
Jul 16, 2026
Contributor
Author
|
Closed by mistake; reopening |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces improvements to the transaction sublog access calculation for AOF sharded logging and adds a new recovery test for sharded AOF transactions. It also makes several test infrastructure updates, including project references, test port assignments, and setup code.
AOF sharded transaction improvements:
TransactionManager.csnow useskeyEntries(populated in both standalone and cluster modes) instead oftxnKeysParseState(which is only filled during cluster slot verification), ensuring correct sublog routing for all modes. (ComputeSublogAccessVector,TransactionManager.cs)Countproperty andGetKeyHash(int index)method toTxnKeyEntry, providing direct access to key hashes for sublog routing. (TxnKeyEntry.cs)Testing and infrastructure:
AofShardedTxnRecoveryTests, to verify that implicit transactions (like RENAME) survive AOF recovery with multiple physical sublogs. (AofShardedTxnRecoveryTests.cs)BfTreeInterop.testandGarnet.test. (BfTreeInterop.test.csproj,Garnet.test.csproj) [1] [2]BfTreeInterop.testand corresponding setup code. (TestUtils.cs,TestProjectSetup.cs) [1] [2]