[JacksonYao287]

this PR only add add_missing_pg_scrub_superblks

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 82.14286% with 5 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v4.x@fff21f7). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/lib/homestore_backend/hs_pg_manager.cpp	57.14%	2 Missing and 1 partial ⚠️
src/lib/homestore_backend/scrub_manager.cpp	85.71%	0 Missing and 1 partial ⚠️
...lib/homestore_backend/snapshot_receive_handler.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/v4.x     #480   +/-   ##
==============================================
  Coverage               ?   54.40%           
==============================================
  Files                  ?       39           
  Lines                  ?     6907           
  Branches               ?      943           
==============================================
  Hits                   ?     3758           
  Misses                 ?     2742           
  Partials               ?      407           

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

[xiaoxichen]

1. PG Move will not carry over the pg_scrub_superblock.
2. The lifecycle of pg_srcrub_superblock is confusing
   • There is a getter get_scrub_superblk but no reference
   • the only reader now submit_scrub_task which checks sb exists and fails if not
   • The writer side save_scrub_superblk tried to create a scrub_sb if not present.

Can we move reader to use get_scrub_superblk and move the create_sb_if_not_presnet into get_scrub_superblk ?

[JacksonYao287]

PG Move will not carry over the pg_scrub_superblock

yes, it will just create a new pg_scrub_superblock in the br receiver since local_create_pg will be called there. actually, this comes back to the previous discussion, how to sync pg_scrub_superblock across the pg?
1 a separate dedicated rpc call
2 through raft.

the solution is not finalized yet, so I did not consider too much on this point here. the intention of this pr is to make scrubber work normally, so we can trigger scrub through restful api.

The lifecycle of pg_srcrub_superblock is confusing

1 get_scrub_superblk is also used in UT to check if the scrub_superblk exists. if we put create_sb_if_not_presnet into get_scrub_superblk, we can not make this check since the returned superblk can be the one exists already or newly created.

2 save_scrub_superblk is just like std::map#insert_or_update,

[Besroy]

generally lgtm, just curious about: when the PG is destroyed, what will happen on the scrub_superblk? What if SM crashed during PG destory (so after restart the PG sb exist but will remove later)

[xiaoxichen]

if we put create_sb_if_not_presnet into get_scrub_superblk, we can not make this check since the returned superblk can be the one exists already or newly created.

I am trying to propose the behavior of [] access to an std::map that if the key doesnt exists, the default constructor is called and returned. UT can be adjusted accordingly. The discussion regarding how to sync pg_scrub_superblock can remains open, as the impact of missing pg_scrub_superblock reduced from "cannot do scrub" to "will schedule unnecessary scrub task".

We dont need to rushing into v4 as well unless we get an emergent fix again to have 0.5.7.1

[JacksonYao287]

What if SM crashed during PG destory (so after restart the PG sb exist but will remove later)

scrub_super_blk will be destroyed when pg is destroyed , pls see

HomeObject/src/lib/homestore_backend/hs_pg_manager.cpp

Line 765 in d5414ba

if (scrub_mgr_) scrub_mgr_->remove_pg(pg_id);

but, this is a good catch. I can put destroy_scrub_superblk into destroy_pg_resource, so that if SM crashed during PG destory, the related scrub_superblk will be destroyed as those pg resource.

We dont need to rushing into v4 as well unless we get an emergent fix again to have 0.5.7.1

IMO, the behavior of how to access scrub_super_blk does not matter too much, since it is not in the critical scrub path.

yes, we have no rush in v4. let`s leave this PR here. i have created a new PR for integration test. we can cycle back on this after the integration test is completed, maybe it will be more clear for us on how should we handle this part then.