Skip to content

feat(node): mark certify events persisted immediately and recover blobs from the pending-recovery table - #3668

Open
halfprice wants to merge 1 commit into
zhewu/pending-recover-blobs-tablefrom
zhewu/record-recovering-blob-and-always-mark-event-persisted
Open

feat(node): mark certify events persisted immediately and recover blobs from the pending-recovery table#3668
halfprice wants to merge 1 commit into
zhewu/pending-recover-blobs-tablefrom
zhewu/record-recovering-blob-and-always-mark-event-persisted

Conversation

@halfprice

Copy link
Copy Markdown
Collaborator

Description

Second PR of the stack on top of #3667.

A certify event that needs a blob sync no longer hands its event handle into the sync task. Holding the handle pinned the persisted event cursor on one slow or stuck recovery: the node kept processing events but could not persist progress, a restart replayed the whole tail (hours under recent production load), and a stall across two epochs escalated into a full recovery-mode scan.

With this change:

  • The certify event durably inserts a record into the pending-recovery table (before the event is marked complete, so a crash between the two writes replays the event and re-inserts idempotently), is marked persisted immediately, and wakes the executor.
  • A background executor on BlobSyncHandler drains the table: records are processed in event order, syncs are bounded by max_concurrent_pending_recoveries, and a record is deleted only when its sync succeeds or the blob is retired. A sync that cannot finish occupies one concurrency slot without blocking other records.
  • Retirement events (InvalidBlobID, and BlobDeleted when the blob is no longer certified) delete the record before cancelling the in-flight sync, so the executor cannot resurrect data. Expired blobs are cleaned up lazily by the executor's certification check.
  • During catch-up, certify events are fast-forwarded without records, as before; the executor parks until catch-up completes.

As a result, the persisted cursor tracks the event tip, restarts resume recovery from the table instead of replaying events, and the pending-record count is visible in metrics and the health endpoint (pendingRecoverBlobCount).

Test plan

  • certified_event_persisted_while_blob_recovery_pending: the certify event is persisted while an unrecoverable blob's sync runs; invalidation deletes the record and cancels the sync.
  • blob_deleted_event_deletes_pending_recovery_record.
  • advances_cursor_past_incomplete_blobs_with_pending_record (rewritten from does_not_advance_cursor_past_incomplete_blobs, which asserted the now-inverted invariant); also exercises that a stuck sync does not block later records.
  • Simtest test_pending_recovery_survives_node_restart: records survive two crash/restart cycles with the cursor at the tip, then drain once recovery is unblocked.
  • Regression: full walrus-service suite (684 tests), test_lagging_node_recovery, workspace clippy.

…bs from the pending-recovery table

A certify event that needs a blob sync no longer holds its event handle
inside the sync task, which pinned the persisted event cursor on one
slow or stuck recovery and forced a full event replay after a restart.
Instead, the event durably records the blob in the pending-recovery
table, is marked complete immediately, and a background executor on
BlobSyncHandler drains the table: records are synced in event order,
bounded by a config knob, and deleted only when the sync succeeds or
the blob is retired. A sync that cannot finish occupies one concurrency
slot without blocking other records. Retirement events delete the
record before cancelling the sync so the executor cannot resurrect
data.

The persisted cursor now tracks the event tip, restarts resume recovery
from the table instead of replaying events, and the pending-record
count is exposed through metrics and the health endpoint.
@github-actions

Copy link
Copy Markdown
Contributor

Warning: This PR modifies one of the OpenAPI files. Please consider the
following:

  • Make sure the API changes are backwards compatible.
  • Make sure to follow existing conventions for any added parameters, requests, and
    responses.
  • Make sure the generated HTML files do not contain errors.
  • This probably requires release notes and maybe changes to the documentation.

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.

1 participant