SDSTOR-20656: Added wbc epochs and UT

[nnastonen]

0. Create homestore with chunk selector
1. Create btree1 and populate it
2. Nodes get into wbc cache
3. Destroy btree1 (wbc purge is not done, also volume's chunk bitmap is not cleared)
4. Create btree2 and populate it
5. Nodes get into wbc cache and we get cache corruption, duplicate inserts

This change adds a generation counter per chunk to invalidate stale WBC entries without scanning or deleting them one by one when custom chunk selector is in use.

When a btree is destroyed with chunk selector enabled, its chunks can be reset and reused by a later btree. The problem is that old WBC entries may still exist for the same BlkIds. When a new btree reuses those BlkIds, WBC can see the old cached entry and treat the new allocation as a duplicate insert, or return stale data.

Instead, each chunk now has a monotonically increasing epoch. Every IndexBuffer records the epoch of the chunk it belongs to. On chunk reset, the chunk epoch is bumped. After that, any cached node whose stored epoch no longer matches the chunk's current epoch is treated as stale and evicted lazily on the next cache access or allocation.

This gives us O(1) invalidation at chunk-reset time, avoids walking millions of WBC entries during destroy, and makes BlkId reuse safe after chunk reset.

If chunk selector is not in use and chunk epochs are never bumped, the stale-entry checks are effectively no-ops.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 38.29787% with 29 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v7.x@7c1d614). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/lib/index/wb_cache.cpp	34.88%	14 Missing and 14 partials ⚠️
src/lib/device/vchunk.cpp	0.00%	1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@              Coverage Diff               @@
##             stable/v7.x     #904   +/-   ##
==============================================
  Coverage               ?   48.30%           
==============================================
  Files                  ?      110           
  Lines                  ?    13147           
  Branches               ?     6339           
==============================================
  Hits                   ?     6351           
  Misses                 ?     2563           
  Partials               ?     4233           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[shosseinimotlagh]

what is the memory increase due to the introduction the notion of epoch?

[JacksonYao287]

according to this PR, if we call chunk->reser_blk_allocator, the epoch will be bumped. this is a behavior change. chunk is not only used by index service, it is also used by other services in homestore. other service or upper layer(like homeobject) will call chunk->reset() in there logic directly. Although epoch brings no impact as of now, not sure will this happen in the future.

IMO, it`s better to not offload checking stale blk to wb_cache itself. it is the responsibility of upper layer to explicitly mark the blk stale by free_node(destory_btree will call free_node).

I don`t have a very strong opinion of this PR. I am not familiar with nublox code. if we can manage to do this in volume destroy phase, it will be better. do we have any workaround in nublox?

[JacksonYao287]

assuming two threads try read_buf for the same blkid simultaneously. if the blkid does not exist in wb_cache, the two threads will read this blk from vdev and then try to insert it into wb_cache(line 201, m_cache.insert(node);). eventually, only one thread will succeed, and the other will retry and will succeed get the blk from wb_cache. this is an expected scenario.

according to the changes here(especially line 204), if the thread , which failed at line 201, succeeds to get the blkid(since the other thread has already inserted it into cache), and this node is not a stale_node(probably happens), it will hit line 211 and lead to assert failure.

[nnastonen]

Thank you, very good observation. Fixed.

[shosseinimotlagh]

@nnastonen would you please check if this happened. IIRC after acquiring the lock the other one will get the refresh lock and this scenario shouldnt happen. Please validate before change.

[JacksonYao287]

suggest rename m_wbc_epoch to m_epoch, since chunk in not only used by index_service, it is also other services in homestore and these services have no idea of what wbc is.

[nnastonen]

Since epochs are related only to wbc, I'd keep the name.

[shosseinimotlagh]

I agree with Jie too. this should be generic.

[raakella1]

I have no additional comments to the ones already posted here. You can merge after resolving the existing ones

[shosseinimotlagh]

for each stale buffer of the current chunk, does this check cause performance degradation ?

[nnastonen]

In my microbenchmark, creating and re-creating a btree with ~200k nodes and ~100k stale cache hits took about the same time. No measurable degradation.

[JacksonYao287]

I have no more comments for this PR, but I would suggest to think through again if we have any workaround in nublox side to destroy btree gracefully and not touch the wbcache/chunk logic for this specific case. theoretically, the destroy_btree should be called for a btree if we do want to destroy it. offloading this to wbcache/chunk seems a little wired.

if you think this change is indeed necessary for nublox case after thinking through, you can merge it.

[shosseinimotlagh]

LGTM, although still against bump_wbc_epoch in the chunk class . Leave it to author to make decision.

[xiaoxichen]

I think I need more context on the intent of this change...

1. it doesnt reduce works, those entries in WBC still needed to be cleanup , it just spread the time cost by cleanup to future then makes the index destroy faster, how much improvement can we get and why it is matters?

2. addition to #1, an addition cost of check on each request had to be introduced due to lazy cleanup . Yes it should be minor as it just an atomic read (with barrier) however it needs to be reasoned together with #1, what it the gain from this change?

3. There is still a racing in flushing WBC vs chunk reset. The epoch checks currently protect cache lookup and allocation, but not dirty/in-flight WBC writes. A buffer can be selected for flush, then its chunk can be reset and reused, after which the old async write may overwrite the new owner’s data.

[xiaoxichen]

A corrupting interleaving is:

1. Old buffer for  (chunk X, block Y)  is selected for flush.
2. Chunk X is reset and its epoch increments.
3. Another btree allocates and writes block Y.
4. The old WBC write is submitted or completes afterward, overwriting the new btree’s node.