SDSTOR-24093: Duplicate inserts in wb_cache

[nnastonen]

Fix for "Duplicate inserts in wb_cache" bug.

In async_cp_flush(), m_vdev->cp_flush() is called only for cp_ctx->id() == 0.

That design misses the case where recovery repaired allocator state but left no dirty index buffers.

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v7.x@0bdc47f). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/lib/index/wb_cache.cpp	50.00%	0 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@              Coverage Diff               @@
##             stable/v7.x     #902   +/-   ##
==============================================
  Coverage               ?   48.41%           
==============================================
  Files                  ?      110           
  Lines                  ?    12984           
  Branches               ?     6239           
==============================================
  Hits                   ?     6286           
  Misses                 ?     2546           
  Partials               ?     4152           

☔ 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.

[JacksonYao287]

thanks @nnastonen for this PR, the RCA looks good to me. I have several comments.

1 pls bump conan version.

2 this PR only handles the blk_commit case, but not handle the blk_free case, which also need to be persisted by vdev->cp_flush. so , my suggeesion is if there is any blk_commit or blk_free in a cp, we need to call vdev->cp_flush in index cp_flush. actually , if there is any blk_commit or free in a cp, we need to trigger vdev->cp_flush, no matter the dirty list is empty

3 it is better to have a UT for this fix.

[JacksonYao287]

I would suggest to add a HS_REL_ASSERT(alloc_status == BlkAllocStatus::SUCCESS) to not ignore any failure when committing blk, since it will be dangerous to go ahead with an uncommitted blk here

[nnastonen]

Before this PR the result of m_vdev->commit_blk() was ignored and with the flow covered by this PR, unsuccessful status will not trigger the flush. I think fixing the error-path is a different concern and should be documented and implemented separately.

[JacksonYao287]

sure。 I am still thinking that since vdev->cp_flush is very light-weight, and for those non-dirtied blk_allocator ， it`s no-op.

so, I think it will be simpler for this PR to always call m_vdev->cp_flush() in async_cp_flush（no need to check allocator_state_changed）(even if cp_ctx->id() is not zero when there is not any dirty buffer). it will be just several line changes and can cover all cases

[nnastonen]

Addressed.

[JacksonYao287]

@nnastonen thank you for the update, the changes looks good to me.
before merging, pls also update conan version in conan.py from 7.5.15 to 7.5.16 ， otherwise the check version work follow will fail.

@shosseinimotlagh do you have any input?

[JacksonYao287]

LGTM

[shosseinimotlagh]

This PR looks good to me! Thank you Niko for addressing it and thank you Jie to review it