Skip to content

SDSTOR-21863: Revert AppendBlkAllocator Reset logic#886

Open
Besroy wants to merge 1 commit into
eBay:stable/v7.xfrom
Besroy:revert_append_blk_allocator_reset
Open

SDSTOR-21863: Revert AppendBlkAllocator Reset logic#886
Besroy wants to merge 1 commit into
eBay:stable/v7.xfrom
Besroy:revert_append_blk_allocator_reset

Conversation

@Besroy
Copy link
Copy Markdown
Contributor

@Besroy Besroy commented May 15, 2026

to prevent race condition between upper layer reset and self operation on it (cp_flush and free)

Fix eBay/HomeObject#401

Copy link
Copy Markdown

Copilot AI left a comment

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 changes AppendBlkAllocator reset behavior to reset in-place (instead of destroy + recreate) to eliminate a race between reset_block_allocator() and concurrent operations (cp_flush, free) as described in HomeObject issue #401.

Changes:

  • Make AppendBlkAllocator::reset() an in-place reset that preserves the superblock/handler and reinitializes in-memory counters.
  • Skip creating a new allocator instance for append allocators during chunk allocator reset in VirtualDev::reset_chunk_blk_allocator().
  • Update and extend prerelease data-service tests to cover the free()-after-reset()-before-cp_flush() scenario.
  • Bump Conan package version to 7.5.9.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/tests/test_data_service.cpp Updates scenario descriptions and adds a new prerelease regression test for the reset/free/cp_flush race.
src/lib/device/virtual_dev.cpp Skips allocator instance replacement for append allocators during chunk reset; minor comment cleanup in cp_flush.
src/lib/blkalloc/append_blk_allocator.cpp Implements in-place reset logic for AppendBlkAllocator and simplifies is_blk_alloced_on_disk().
conanfile.py Increments the package version from 7.5.8 to 7.5.9.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/tests/test_data_service.cpp Outdated
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 15, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v7.x@0bc81f1). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/lib/blkalloc/append_blk_allocator.cpp 80.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     #886   +/-   ##
==============================================
  Coverage               ?   48.18%           
==============================================
  Files                  ?      110           
  Lines                  ?    12938           
  Branches               ?     6219           
==============================================
  Hits                   ?     6234           
  Misses                 ?     2577           
  Partials               ?     4127           

☔ View full report in Codecov by Sentry.
📢 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.

…ondition between upper layer reset and self operation on it (cp_flush and free)

Fix eBay/HomeObject#401

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Besroy Besroy force-pushed the revert_append_blk_allocator_reset branch from 2661a6e to ceaba2c Compare May 15, 2026 09:49
Copy link
Copy Markdown
Contributor

@JacksonYao287 JacksonYao287 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Race Condition Between GC Chunk Reset and Background Expired ReplReq Cleanup

4 participants