Repository navigation
atelet: Report background uploads and detaches to ate-api-server - #2810
Conversation
78fe309 to
de9da47
Compare
| if a.LatestLocal == nil || !a.uploadNeeded(*a.LatestLocal) { | ||
| a.UndurableSince = time.Time{} | ||
| } | ||
| t.cancelReportLocked(a) |
There was a problem hiding this comment.
🤖 question 🟢 – This also cancels a pending detach report when a promote follows a background detach before the report was acknowledged. That promote finds the snapshot already durable and returns a state with no latest_local. The control plane applies a PromoteSnapshot response with applyCheckpointState, which records latest_durable but does not treat a missing latest_local as a detach. The actor would then stay pinned to a node that no longer holds its snapshot, until its next suspend.
Nothing on main calls PromoteSnapshot today, so this only matters once a caller such as the planned Tag flow promotes a paused actor. Should the report stay due when LatestLocal is nil, or should the control plane apply the detach from a promote's response?
There was a problem hiding this comment.
Fixed. CommitPromoted now cancels a pending report only when LatestLocal is non-nil, so a pending detach report stays due when PromoteSnapshot runs after a background detach.
| // The control plane answered; only this actor conflicted. | ||
| t.reporting.queueRetries.Forget("") | ||
| } else { | ||
| queueDelay = jitteredReportDelay(t.reporting.queueRetries.When("")) |
There was a problem hiding this comment.
🤖 question 🟢 – queueRetries counts failures across all actors and resets only on a success or a refusal. A short outage that fails a handful of reports takes the pause to the 5m cap. With the default single worker, reports can then stay paused for up to 5m after ate-api-server is back. Is that recovery delay intended? A lower cap for the queue-wide limiter than for the per-actor one would shorten it.
There was a problem hiding this comment.
Fixed. Capped queueRetries at 30s (maxReportQueueRetryDelay) while keeping the per-actor cap at 5m, so the report queue resumes within 30s after ate-api-server recovers.
| // upload_due_time, which the control plane does not record. | ||
| func sameReport(a, b *ateletpb.ActorSnapshotState) bool { | ||
| return proto.Equal(a.GetLatestLocal(), b.GetLatestLocal()) && | ||
| proto.Equal(a.GetUploading(), b.GetUploading()) && |
There was a problem hiding this comment.
🤖 should-fix 🟡 – sameReport compares Uploading, which changes when a background upload starts or fails without making a report due. If a newer snapshot starts uploading while a report is in flight, endReport sees a different state and queues another report right away, even after a control-plane refusal. Should we compare only LatestLocal and LatestDurable here?
There was a problem hiding this comment.
Fixed. sameReport now compares only LatestLocal and LatestDurable, ignoring Uploading as well as UploadDueTime.
de9da47 to
a06eb58
Compare
atelet uploads and detaches an actor's snapshots on its own timers, and ate-api-server learned of them only from the state a later Checkpoint or PromoteSnapshot returned. atelet now calls WorkerService.ReportActorSnapshotState with the actor's snapshot state after a background upload makes a newer snapshot durable, and after a background detach deletes the actor's local snapshots. A report that fails is retried with a backoff until ate-api-server acknowledges it, refuses it with NotFound, FailedPrecondition, InvalidArgument, or Unimplemented, or the retry window expires. A pending report is kept in the actor's snapshot state file, so it is still sent after an atelet restart. A Checkpoint, PromoteSnapshot, or Detach call returns the latest state in its response, so it cancels any pending report and queues none of its own.
a06eb58 to
81cfe20
Compare
atelet uploads and detaches an actor's snapshots on its own timers, and ate-api-server learned of them only from the state a later Checkpoint or PromoteSnapshot returned. atelet now calls
WorkerService.ReportActorSnapshotState with the actor's snapshot state after a background upload makes a newer snapshot durable, and after a background detach deletes the actor's local snapshots.
A report that fails is retried with a backoff until ate-api-server acknowledges it, refuses it with NotFound, FailedPrecondition, InvalidArgument, or Unimplemented, or the retry window expires. A pending report is kept in the actor's snapshot state file, so it is still sent after an atelet restart.
A Checkpoint, PromoteSnapshot, or Detach call returns the latest state in its response, so it cancels any pending report and queues none of its own.
Part of #2069 and #2571