Sync scrub superblk timestamps between leader and followers via RPC by JacksonYao287 · Pull Request #475 · eBay/HomeObject

JacksonYao287 · 2026-09-30T07:38:03Z

Followers previously had no visibility into a PG's last_deep_scrub_timestamp/ last_shallow_scrub_timestamp, which the leader persists locally after each scrub. Add a PUSH_SCRUB_TIMESTAMP data RPC channel and a periodic leader-side timer that pushes these timestamps to followers; each side keeps the newer value per field and the response is merged back into the leader.

after this is merged in this branch , will cherry-pick it to dev/5.x

Followers previously had no visibility into a PG's last_deep_scrub_timestamp/ last_shallow_scrub_timestamp, which the leader persists locally after each scrub. Add a PUSH_SCRUB_SUPERBLK data RPC channel and a periodic leader-side timer that pushes these timestamps to followers; each side keeps the newer value per field and the response is merged back into the leader.

codecov-commenter · 2026-09-30T09:53:57Z

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

Codecov Report

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

Files with missing lines	Patch %	Lines
src/lib/homestore_backend/scrub_manager.cpp	76.59%	13 Missing and 9 partials ⚠️
src/lib/homestore_backend/hs_pg_manager.cpp	50.00%	10 Missing and 4 partials ⚠️
...ib/homestore_backend/replication_state_machine.cpp	50.00%	0 Missing and 1 partial ⚠️
src/lib/homestore_backend/scrub_manager.hpp	92.30%	1 Missing ⚠️
...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     #475   +/-   ##
==============================================
  Coverage               ?   54.57%           
==============================================
  Files                  ?       39           
  Lines                  ?     6999           
  Branches               ?      961           
==============================================
  Hits                   ?     3820           
  Misses                 ?     2764           
  Partials               ?      415

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

Besroy · 2026-10-08T07:19:20Z

        m_retry_timer_hdl = iomanager.schedule_thread_timer(5ull * 1000 * 1000 * 1000, true, nullptr /*cookie*/,
                                                            [this](void*) { check_scrub_timeouts(); });
+        // TODO: make the interval configurable, for now set it to 30 seconds
+        m_push_scrub_timestamp_timer_hdl =


Does this mean that after this commit, we will start a periodic sync-up job by default, and on 4.x the value passed around will always be 0 because scrub is not enabled there?

Generally LGTM, but I am curious why we need to sync every 30 seconds. The scrub interval is usually on the order of days or even weeks, so do we really need such frequent synchronization? Also, if the follower has already received the value, do we still need to keep syncing it repeatedly?

Does this mean that after this commit, we will start a periodic sync-up job by default, and on 4.x the value passed around will always be 0 because scrub is not enabled there?

no , if scrubber is disabled, scrubber->start() will not be called and this timer will not be started.

do we really need such frequent synchronization? Also, if the follower has already received the value, do we still need to keep syncing it repeatedly?

1 30 seconds , 1 minute , 1 hour or even 1 day will not make much difference for a single rpc call.

2 we need keep syncing it repeatedly since pg member will probably change(replace member) , we need let the new member be aware of this timestamp. besides, this rpc might timeout.

3 if leader crash before it syncs the latest timestamps to followers, then we lose it in this pg before it comes back. the longer interval is , the bigger possibility we have to lose it.

generally speaking, a single rpc call is very cheap here, no need to pay too much attention on the interval(frequency).

Besroy · 2026-10-08T07:22:46Z

+    scrub_timestamp_info sync_scrub_timestamp(const pg_id_t pg_id, const scrub_timestamp_info& incoming);
+
+    // leader-only: periodically push last_deep_scrub_timestamp/last_shallow_scrub_timestamp to followers of every pg
+    // this node leads, and merge back whatever the followers report as newer. Exposed publicly (in addition to being


Could you explain why we need to merge back here? My understanding is that the leader is the source of truth, so I am not fully clear on why we would update the state again based on data from the follower.

let`s say we have A,B,C. A is leader.

after a scrub , A has the latest scrub timestamp. before this timestamp is synced to followers , leader switch happen and B becomes the new leader. B now does not have the latest scrub timestamp. when B syncs its scrub timestamp to A, A should echo back its timestamp to B, and B update its own timestamp.

A should not update its timestamp with the stale timestamp from B, even if B is leader now.

so , generall speaking, we should always keep the latest scrub timestamp across the pg, it is independent of leadership.

JacksonYao287

@Besroy thanks for the review, I have added my comments

JacksonYao287 · 2026-10-08T08:13:30Z

+    scrub_timestamp_info sync_scrub_timestamp(const pg_id_t pg_id, const scrub_timestamp_info& incoming);
+
+    // leader-only: periodically push last_deep_scrub_timestamp/last_shallow_scrub_timestamp to followers of every pg
+    // this node leads, and merge back whatever the followers report as newer. Exposed publicly (in addition to being


let`s say we have A,B,C. A is leader.

after a scrub , A has the latest scrub timestamp. before this timestamp is synced to followers , leader switch happen and B becomes the new leader. B now does not have the latest scrub timestamp. when B syncs its scrub timestamp to A, A should echo back its timestamp to B, and B update its own timestamp.

A should not update its timestamp with the stale timestamp from B, even if B is leader now.

so , generall speaking, we should always keep the latest scrub timestamp across the pg, it is independent of leadership.

JacksonYao287 · 2026-10-08T08:29:21Z

        m_retry_timer_hdl = iomanager.schedule_thread_timer(5ull * 1000 * 1000 * 1000, true, nullptr /*cookie*/,
                                                            [this](void*) { check_scrub_timeouts(); });
+        // TODO: make the interval configurable, for now set it to 30 seconds
+        m_push_scrub_timestamp_timer_hdl =


Does this mean that after this commit, we will start a periodic sync-up job by default, and on 4.x the value passed around will always be 0 because scrub is not enabled there?

no , if scrubber is disabled, scrubber->start() will not be called and this timer will not be started.

do we really need such frequent synchronization? Also, if the follower has already received the value, do we still need to keep syncing it repeatedly?

1 30 seconds , 1 minute , 1 hour or even 1 day will not make much difference for a single rpc call.

2 we need keep syncing it repeatedly since pg member will probably change(replace member) , we need let the new member be aware of this timestamp. besides, this rpc might timeout.

3 if leader crash before it syncs the latest timestamps to followers, then we lose it in this pg before it comes back. the longer interval is , the bigger possibility we have to lose it.

generally speaking, a single rpc call is very cheap here, no need to pay too much attention on the interval(frequency).

JacksonYao287 · 2026-10-12T07:57:25Z

@xiaoxichen can you also take a look?

Besroy · 2026-10-14T06:43:23Z

@JacksonYao287 Thanks for the detailed explanation, LGTM.

One edge case I'm not sure if needs to be considered (maybe in the future?):
After a leader switch, there's a window before the new leader's first push_scrub_timestamp_to_followers() fires. If scan_pg_for_scrub happens to run in that window, the new leader may trigger a scrub based on stale timestamps — potentially kicking off an unnecessary hours-long scrub. Under repeated leader flip-flops this could become more disruptive.
If this is a concern, maybe a "new leader grace period" before the first eligibility check, or a flag that requires at least one timestamp sync to complete before scrub eligibility can help.

JacksonYao287 · 2026-10-14T10:03:10Z

If this is a concern, maybe a "new leader grace period" before the first eligibility check, or a flag that requires at least one timestamp sync to complete before scrub eligibility can help.

@Besroy thanks, this is a good point. I can add a push_scrub_timestamp_to_followers to on_become_leader, so that the new leader will sync scrub_timestamp as soon as it is elected

JacksonYao287 · 2026-10-14T10:26:43Z

@Besroy thanks for the suggestion, I have make the change , ptal

xiaoxichen · 2026-10-15T02:16:46Z

Let me first ensure I read it correctly.
The intent of this change is to keep the last_scrub timestamp consistent across replicas, so that once leader switch happens follower can make better scheduling on which PG should scrub next.

The intent part is LGTM.

Regarding the implementation, I have a few questions:

The timestamp only needed to be update when a scrub finishes.
Can we go through raft and generalized into UpdatePGMetaData ?

The information flow is naturally from leader to every member. If we go through raft, the consistency is ensured, no corner cases like leader switch, new leader join needed to be considered. Then we can make it only once that make a raft proposal to update PGMeta inside which update the scrub timestamp. Even if there are some corner cases, the worst case is just we lost track of "this" single PG scan timestamp

It can be further extended to sync other via same message or different message, for example potentially sync the deep-scrub progress , or the scrub result...extra, extra

JacksonYao287 · 2026-10-15T03:23:48Z

@xiaoxichen thanks for the reivew.

The timestamp only needed to be update when a scrub finishes.

if leader does not change , yes . but , as I mentioned above, after leader switch or replace member , the new leader or the new member also need to be aware of this latest timestamp. so I introduce a periodical rpc call to sync this across the entire pg.

Can we go through raft and generalized into UpdatePGMetaData ?

this is actually my initial idea. but, if we add this to raft , it will be much complicated since we need to think carefully about how to handle this in BR, rollback, etc. that might bring a lot of changes and unknown issues to the well tested and stable raft path. so IMO, do not touch raft path unless it is really indeed necessary. in this PR , I use a single rpc call to achieve that goal. it works just like gossip, so we don`t need to worry about the corner cases that might be brought by raft.

I have added a version number to this message (scrub_timestamp_info), so that we can extend it to v2 or v3.. to add more info in the future if we want. maybe we can change it to flatbuffer?

The information flow is naturally from leader to every member

this not always true here, pls see #475 (comment)

JacksonYao287 requested review from Besroy and xiaoxichen September 30, 2026 08:49

JacksonYao287 force-pushed the add-rpc-to-sync-scrub-super-blk branch from cdd62c2 to 9560396 Compare September 30, 2026 09:15

add lock for scrub superblk

748f19b

Besroy reviewed Oct 8, 2026

View reviewed changes

Comment thread src/lib/homestore_backend/scrub_manager.hpp

Besroy reviewed Oct 8, 2026

View reviewed changes

JacksonYao287 commented Oct 8, 2026

View reviewed changes

JacksonYao287 requested a review from Besroy October 10, 2026 06:41

add crc and version in scrub_timestamp_info

4c88263

address comments

0e1f525

JacksonYao287 mentioned this pull request Oct 19, 2026

Expose data corruptted api for scrubber integration test #479

Closed

Conversation

JacksonYao287 commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Besroy Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Besroy Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JacksonYao287 Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 commented Oct 12, 2026

Uh oh!

Besroy commented Oct 14, 2026

Uh oh!

JacksonYao287 commented Oct 14, 2026

Uh oh!

JacksonYao287 commented Oct 14, 2026

Uh oh!

xiaoxichen commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JacksonYao287 commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JacksonYao287 commented Sep 30, 2026 •

edited

Loading

codecov-commenter commented Sep 30, 2026 •

edited

Loading

xiaoxichen commented Oct 15, 2026 •

edited

Loading