Skip to content

fix(txmetacache): restore delete correctness for native and trimmed #836

Merged
gokutheengineer merged 2 commits into
bsv-blockchain:mainfrom
gokutheengineer:fix/txmetacache-issue-2-del-lock-invariant
May 14, 2026
Merged

fix(txmetacache): restore delete correctness for native and trimmed #836
gokutheengineer merged 2 commits into
bsv-blockchain:mainfrom
gokutheengineer:fix/txmetacache-issue-2-del-lock-invariant

Conversation

@gokutheengineer

@gokutheengineer gokutheengineer commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

  • fixed Del() correctness for bucketNative and bucketTrimmed by deleting from the correct shard while keeping shard length accounting accurate.
  • For item 3 (freeChunks lock invariant), added explicit doc comments:
    • bucketNative uses freeChunksLock
    • bucketTrimmed / bucketUnallocated require caller-held b.mu
  • Added focused tests to lock in behavior across bucket types:
    • after Del(k): Has(k) == false
    • Get(k) fails
    • map size shrinks after delete
    • concurrent Set/Get/Del/Reset stress test

Validation

  • go test ./stores/txmetacache -run '^$' -count=1 (compile-only)
  • Full test suite not run (per request).

@github-actions

github-actions Bot commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

Summary: This PR correctly fixes delete operations for Native and Trimmed bucket types. The previous implementation bypassed the swiss map API and corrupted the length counter. The new shard-rebuild approach properly maintains length correctness.

Findings:

✅ Correctness: Delete implementation properly synchronized with locks. The shard rebuild approach correctly maintains the length counter that was broken by the old delete() call on the internal map.

✅ Test Coverage: Comprehensive test additions verify delete correctness (Has returns false, Get fails, map size shrinks) and include concurrent stress testing across all bucket types.

✅ Documentation: Lock invariants are now clearly documented with inline comments explaining which lock protects each bucket type.

Minor Performance Note: Both deleteFromNativeSplitMapShard and deleteFromSwissSplitMapShard allocate the rebuilt shard with capacity shard.Length() but will hold one fewer item after deletion. Consider using shard.Length() - 1 for the capacity to avoid slight over-allocation. This is a micro-optimization, not a correctness issue.

Verification Required (per AGENTS.md): Since the PR description states "Full test suite not run", ensure the following pass before merge:

go test ./stores/txmetacache -race -count=1
go test ./...
go vet ./...
golangci-lint run

Previous Issues: All previously flagged race condition concerns have been resolved and were incorrect.

github-actions[bot]
github-actions Bot reviewed May 8, 2026
View reviewed changes
Comment thread stores/txmetacache/improved_cache.go
github-actions[bot]
github-actions Bot reviewed May 8, 2026
View reviewed changes
Comment thread stores/txmetacache/improved_cache.go
github-actions[bot]
github-actions Bot reviewed May 8, 2026
View reviewed changes
Comment thread stores/txmetacache/improved_cache_test.go
@github-actions

github-actions Bot commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-836 (e67a78f)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 142
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.884µ 1.711µ ~ 0.700
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.26n 59.35n ~ 0.100
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 59.24n 59.55n ~ 0.200
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 59.35n 59.17n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 35.19n 35.53n ~ 1.000
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 59.86n 62.62n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 161.4n 154.5n ~ 0.100
MiningCandidate_Stringify_Short-4 256.1n 256.4n ~ 1.000
MiningCandidate_Stringify_Long-4 1.771µ 1.748µ ~ 0.100
MiningSolution_Stringify-4 915.0n 907.3n ~ 0.400
BlockInfo_MarshalJSON-4 1.748µ 1.745µ ~ 0.600
NewFromBytes-4 128.4n 128.5n ~ 1.000
Mine_EasyDifficulty-4 66.73µ 67.14µ ~ 0.100
Mine_WithAddress-4 7.027µ 6.971µ ~ 0.700
BlockAssembler_AddTx-4 0.02865n 0.02883n ~ 1.000
AddNode-4 10.83 11.06 ~ 0.400
AddNodeWithMap-4 11.03 11.11 ~ 0.400
DiskTxMap_SetIfNotExists-4 3.733µ 3.787µ ~ 0.400
DiskTxMap_SetIfNotExists_Parallel-4 3.572µ 3.562µ ~ 1.000
DiskTxMap_ExistenceOnly-4 343.5n 482.1n ~ 0.100
Queue-4 197.2n 202.3n ~ 0.100
AtomicPointer-4 3.803n 3.770n ~ 1.000
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 922.9µ 917.1µ ~ 1.000
ReorgOptimizations/DedupFilterPipeline/New/10K-4 863.6µ 909.8µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 124.9µ 116.1µ ~ 0.100
ReorgOptimizations/AllMarkFalse/New/10K-4 62.70µ 62.62µ ~ 0.400
ReorgOptimizations/HashSlicePool/Old/10K-4 65.75µ 63.93µ ~ 0.700
ReorgOptimizations/HashSlicePool/New/10K-4 11.78µ 11.24µ ~ 0.100
ReorgOptimizations/NodeFlags/Old/10K-4 5.353µ 5.485µ ~ 0.700
ReorgOptimizations/NodeFlags/New/10K-4 1.831µ 1.835µ ~ 1.000
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 10.98m 11.06m ~ 0.700
ReorgOptimizations/DedupFilterPipeline/New/100K-4 10.22m 10.26m ~ 1.000
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.181m 1.193m ~ 0.100
ReorgOptimizations/AllMarkFalse/New/100K-4 684.8µ 686.6µ ~ 0.700
ReorgOptimizations/HashSlicePool/Old/100K-4 630.6µ 659.6µ ~ 0.700
ReorgOptimizations/HashSlicePool/New/100K-4 318.0µ 312.1µ ~ 0.200
ReorgOptimizations/NodeFlags/Old/100K-4 55.55µ 59.62µ ~ 0.200
ReorgOptimizations/NodeFlags/New/100K-4 20.59µ 20.24µ ~ 0.200
TxMapSetIfNotExists-4 51.69n 51.70n ~ 1.000
TxMapSetIfNotExistsDuplicate-4 38.17n 38.26n ~ 0.700
ChannelSendReceive-4 643.8n 596.8n ~ 0.100
DirectSubtreeAdd/4_per_subtree-4 60.87n 62.68n ~ 0.700
DirectSubtreeAdd/64_per_subtree-4 29.85n 28.92n ~ 0.100
DirectSubtreeAdd/256_per_subtree-4 27.98n 27.80n ~ 0.700
DirectSubtreeAdd/1024_per_subtree-4 26.55n 26.52n ~ 1.000
DirectSubtreeAdd/2048_per_subtree-4 26.18n 26.19n ~ 0.400
SubtreeProcessorAdd/4_per_subtree-4 282.5n 279.0n ~ 0.400
SubtreeProcessorAdd/64_per_subtree-4 273.3n 274.9n ~ 1.000
SubtreeProcessorAdd/256_per_subtree-4 273.9n 275.2n ~ 1.000
SubtreeProcessorAdd/1024_per_subtree-4 266.9n 265.9n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 266.7n 268.7n ~ 0.100
SubtreeProcessorRotate/4_per_subtree-4 270.4n 271.0n ~ 1.000
SubtreeProcessorRotate/64_per_subtree-4 269.6n 268.8n ~ 0.400
SubtreeProcessorRotate/256_per_subtree-4 272.0n 267.5n ~ 0.100
SubtreeProcessorRotate/1024_per_subtree-4 269.6n 267.4n ~ 0.100
SubtreeNodeAddOnly/4_per_subtree-4 55.52n 54.80n ~ 0.100
SubtreeNodeAddOnly/64_per_subtree-4 36.19n 36.00n ~ 0.100
SubtreeNodeAddOnly/256_per_subtree-4 35.48n 34.99n ~ 0.100
SubtreeNodeAddOnly/1024_per_subtree-4 34.58n 34.49n ~ 0.500
SubtreeCreationOnly/4_per_subtree-4 112.5n 109.3n ~ 0.100
SubtreeCreationOnly/64_per_subtree-4 346.0n 345.4n ~ 0.500
SubtreeCreationOnly/256_per_subtree-4 1.206µ 1.197µ ~ 0.400
SubtreeCreationOnly/1024_per_subtree-4 3.817µ 3.909µ ~ 0.200
SubtreeCreationOnly/2048_per_subtree-4 6.717µ 7.041µ ~ 0.100
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 268.6n 267.6n ~ 0.300
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 268.7n 268.0n ~ 0.200
ParallelGetAndSetIfNotExists/1k_nodes-4 549.1µ 540.2µ ~ 0.400
ParallelGetAndSetIfNotExists/10k_nodes-4 1.339m 1.334m ~ 0.400
ParallelGetAndSetIfNotExists/50k_nodes-4 6.683m 6.526m ~ 0.100
ParallelGetAndSetIfNotExists/100k_nodes-4 13.36m 13.22m ~ 0.700
SequentialGetAndSetIfNotExists/1k_nodes-4 616.0µ 617.0µ ~ 1.000
SequentialGetAndSetIfNotExists/10k_nodes-4 2.820m 2.836m ~ 1.000
SequentialGetAndSetIfNotExists/50k_nodes-4 10.92m 10.98m ~ 0.200
SequentialGetAndSetIfNotExists/100k_nodes-4 21.38m 21.17m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 586.2µ 586.9µ ~ 1.000
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 4.566m 4.563m ~ 0.700
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 16.61m 16.75m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 662.5µ 665.0µ ~ 0.700
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 6.223m 6.220m ~ 1.000
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 39.63m 38.94m ~ 0.100
CalcBlockWork-4 494.8n 497.8n ~ 0.100
CalculateWork-4 674.2n 687.9n ~ 1.000
BuildBlockLocatorString_Helpers/Size_10-4 1.357µ 1.362µ ~ 1.000
BuildBlockLocatorString_Helpers/Size_100-4 12.85µ 12.83µ ~ 0.800
BuildBlockLocatorString_Helpers/Size_1000-4 161.8µ 127.5µ ~ 0.700
CatchupWithHeaderCache-4 104.6m 104.9m ~ 0.200
_BufferPoolAllocation/16KB-4 3.499µ 3.402µ ~ 0.400
_BufferPoolAllocation/32KB-4 7.477µ 8.593µ ~ 0.700
_BufferPoolAllocation/64KB-4 15.02µ 16.59µ ~ 0.700
_BufferPoolAllocation/128KB-4 28.69µ 30.90µ ~ 0.100
_BufferPoolAllocation/512KB-4 116.3µ 113.8µ ~ 0.700
_BufferPoolConcurrent/32KB-4 19.14µ 18.43µ ~ 0.700
_BufferPoolConcurrent/64KB-4 30.30µ 28.16µ ~ 0.100
_BufferPoolConcurrent/512KB-4 145.3µ 140.5µ ~ 0.200
_SubtreeDeserializationWithBufferSizes/16KB-4 681.3µ 631.6µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/32KB-4 676.0µ 610.1µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 670.3µ 613.8µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/128KB-4 682.7µ 613.6µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/512KB-4 654.6µ 628.3µ ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/16KB-4 35.30m 35.43m ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/32KB-4 35.40m 35.34m ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/64KB-4 35.40m 35.18m ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/128KB-4 35.07m 35.54m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/512KB-4 34.98m 34.98m ~ 1.000
_PooledVsNonPooled/Pooled-4 737.5n 734.8n ~ 0.100
_PooledVsNonPooled/NonPooled-4 7.147µ 7.155µ ~ 1.000
_MemoryFootprint/Current_512KB_32concurrent-4 6.840µ 6.796µ ~ 0.200
_MemoryFootprint/Proposed_32KB_32concurrent-4 10.70µ 10.04µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 9.654µ 9.440µ ~ 0.100
SubtreeSizes/10k_tx_4_per_subtree-4 1.368m 1.398m ~ 0.400
SubtreeSizes/10k_tx_16_per_subtree-4 328.0µ 328.5µ ~ 0.700
SubtreeSizes/10k_tx_64_per_subtree-4 77.80µ 78.54µ ~ 0.700
SubtreeSizes/10k_tx_256_per_subtree-4 19.81µ 19.51µ ~ 0.100
SubtreeSizes/10k_tx_512_per_subtree-4 9.821µ 9.728µ ~ 0.100
SubtreeSizes/10k_tx_1024_per_subtree-4 4.922µ 4.837µ ~ 0.100
SubtreeSizes/10k_tx_2k_per_subtree-4 2.421µ 2.394µ ~ 0.100
BlockSizeScaling/10k_tx_64_per_subtree-4 77.88µ 76.46µ ~ 0.100
BlockSizeScaling/10k_tx_256_per_subtree-4 19.75µ 19.18µ ~ 0.100
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.897µ 4.776µ ~ 0.100
BlockSizeScaling/50k_tx_64_per_subtree-4 410.7µ 406.4µ ~ 0.700
BlockSizeScaling/50k_tx_256_per_subtree-4 98.57µ 96.33µ ~ 0.100
BlockSizeScaling/50k_tx_1024_per_subtree-4 24.21µ 23.72µ ~ 0.100
SubtreeAllocations/small_subtrees_exists_check-4 167.0µ 162.6µ ~ 0.700
SubtreeAllocations/small_subtrees_data_fetch-4 177.1µ 164.8µ ~ 0.100
SubtreeAllocations/small_subtrees_full_validation-4 340.5µ 332.1µ ~ 0.100
SubtreeAllocations/medium_subtrees_exists_check-4 9.914µ 9.777µ ~ 0.200
SubtreeAllocations/medium_subtrees_data_fetch-4 10.236µ 9.844µ ~ 0.100
SubtreeAllocations/medium_subtrees_full_validation-4 19.76µ 19.37µ ~ 0.100
SubtreeAllocations/large_subtrees_exists_check-4 2.340µ 2.347µ ~ 1.000
SubtreeAllocations/large_subtrees_data_fetch-4 2.483µ 2.383µ ~ 0.100
SubtreeAllocations/large_subtrees_full_validation-4 4.938µ 4.867µ ~ 0.100
_prepareTxsPerLevel-4 400.0m 403.5m ~ 1.000
_prepareTxsPerLevelOrdered-4 3.664m 3.473m ~ 0.100
_prepareTxsPerLevel_Comparison/Original-4 405.5m 398.8m ~ 0.100
_prepareTxsPerLevel_Comparison/Optimized-4 3.502m 3.482m ~ 1.000
StoreBlock_Sequential/BelowCSVHeight-4 319.8µ 319.7µ ~ 1.000
StoreBlock_Sequential/AboveCSVHeight-4 319.6µ 313.4µ ~ 0.100
GetUtxoHashes-4 259.4n 258.5n ~ 1.000
GetUtxoHashes_ManyOutputs-4 47.47µ 43.16µ ~ 0.100
_NewMetaDataFromBytes-4 237.9n 238.0n ~ 0.600
_Bytes-4 622.5n 623.7n ~ 1.000
_MetaBytes-4 570.8n 562.5n ~ 0.100

Threshold: >10% with p < 0.05 | Generated: 2026-05-13 12:55 UTC

@gokutheengineer
fix(txmetacache): restore delete correctness for native and trimmed b…
0237eeb 
…uckets

Rebuild target shards on delete so hash removal updates shard length accounting, and document the intentional freeChunks locking invariants while tightening delete and concurrency tests.
@gokutheengineer gokutheengineer force-pushed the fix/txmetacache-issue-2-del-lock-invariant branch from 2c50037 to 0237eeb Compare May 13, 2026 11:29
@sonarqubecloud

sonarqubecloud Bot commented May 13, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@gokutheengineer gokutheengineer merged commit f145df7 into bsv-blockchain:main May 14, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@icellan icellan icellan approved these changes

@freemans13 freemans13 freemans13 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gokutheengineer @icellan @freemans13