Skip to content

Fix ComputeSublogAccessVector to use keyEntries#1939

Merged
TedHartMS merged 7 commits into
mainfrom
tedhar/sublog-acc-vec
Jul 17, 2026
Merged

Fix ComputeSublogAccessVector to use keyEntries#1939
TedHartMS merged 7 commits into
mainfrom
tedhar/sublog-acc-vec

Conversation

@TedHartMS

Copy link
Copy Markdown
Contributor

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:

  • The computation of the sublog access bitmap in TransactionManager.cs now uses keyEntries (populated in both standalone and cluster modes) instead of txnKeysParseState (which is only filled during cluster slot verification), ensuring correct sublog routing for all modes. (ComputeSublogAccessVector, TransactionManager.cs)
  • Added Count property and GetKeyHash(int index) method to TxnKeyEntry, providing direct access to key hashes for sublog routing. (TxnKeyEntry.cs)

Testing and infrastructure:

  • Added a new test, AofShardedTxnRecoveryTests, to verify that implicit transactions (like RENAME) survive AOF recovery with multiple physical sublogs. (AofShardedTxnRecoveryTests.cs)
  • Updated test project references and internals visibility to enable integration between BfTreeInterop.test and Garnet.test. (BfTreeInterop.test.csproj, Garnet.test.csproj) [1] [2]
  • Introduced a dedicated test port assignment for BfTreeInterop.test and corresponding setup code. (TestUtils.cs, TestProjectSetup.cs) [1] [2]

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ComputeSublogAccessVector to use keyEntries-backed key hashes for sublog routing (instead of cluster-only parse state).
  • Extend TxnKeyEntries with Count and GetKeyHash(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 vazois closed this in #1940 Jul 16, 2026
@TedHartMS

Copy link
Copy Markdown
Contributor Author

Closed by mistake; reopening

@TedHartMS TedHartMS reopened this Jul 17, 2026
@TedHartMS
TedHartMS merged commit 723dbc3 into main Jul 17, 2026
160 of 161 checks passed
@TedHartMS
TedHartMS deleted the tedhar/sublog-acc-vec branch July 17, 2026 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants