Implement Shard lease protocol: RenewShardLease, Deregister, epoch in AssignShards

[Aditya1404Sal]

resolves GOL-448

Shard ownership was push-only: the shard manager told an executor what it held and never heard from
it again, so a wedged executor kept its shards indefinitely. This makes ownership a lease that lapses
unless it is renewed, and gets each shard's epoch to the executor so ticket 5 can fence oplog writes
on it. Every behavioural test is mutation-checked. Draining agents on lease loss and the oplog fence
itself are next steps; #3766 lands after them

What this adds

• RenewShardLease. An executor renews at a third of the time left on its lease (about 20 s on
  the 1 m default), claiming the epoch of every shard it holds. Failed renewals back off exponentially,
  capped by the lease; a lease with no expiry (single-shard mode, the debugging service) never renews.
• Epochs on the wire. A shard's epoch is its ownership generation: advanced when the shard changes
  owner, never on a renewal, so a renewal is idempotent and a lost response is harmless.
• A mismatched claim is renewed and corrected. The response carries the manager's set for that
  executor, which the executor adopts exactly as it would a push. Pushes deliver a change at once;
  renewals guarantee it arrives within a third of the lease even if the push was lost. The manager
  logs the mismatch, which is the one signal that pushes to that executor are not landing.
• A revision on every delivery (RegisterSuccess, AssignShards, ShardLease): the revision
  of the persisted state the set was read from. The executor applies a delivery only if that is at
  least the last one applied, so a push and a renewal that cross on the network cannot leave the older
  set in place.
• A rebalance is stored before any of it is sent. Both halves of the protocol rest on this. A
  delivery names the revision it really lands at instead of predicting one that a renewal persisting
  in the meantime would consume. And a revoke - the one delivery with no revision, so nothing can
  order it against a grant - is only ever sent for a shard the store has already moved, so a renewal
  served in the middle of the fan-out cannot hand it straight back. Epochs are minted in that same
  write, so no executor is ever told an epoch the store does not hold, and a refused write changes
  nothing anywhere.
• One receipt path on the executor for any delivered set: sweep the agents on dropped shards,
  recover the agents on gained ones, announce. Registration, pushes and corrected renewals all go
  through it, so a renewal that narrows the set sweeps exactly as a push would.
• The lease travels as the time remaining (lease_ttl), anchored to the executor's own clock on
  receipt, so the two machines' clocks are never compared and skew cannot lengthen or shorten a lease.
  Decoding is total: an absent, negative or out-of-range value is an error, never a panic and never a
  lease that does not expire.
• A timer in the manager's loop, at a third of the lease, alongside the existing event wake-ups.
  A lapsed lease is reclaimed and its shards re-homed within one tick, even in an idle cluster.
• The self-fence. An executor whose own copy of the lease has lapsed stops admitting work and
  answers ShardingNotReady, which the worker service already answers by refreshing its routing table
  and retrying. Running invocations are not interrupted, and routing checks are never fenced.
• Deregister. A graceful stop, including the SIGTERM an orchestrator sends, hands the lease
  back so the shards move on the next tick rather than after the lease expires.
• AssignShards is a full-replace push carrying epochs, the lease TTL, the revision and the
  shard count. It absorbs SetShardAssignment, which is deleted and its field number reserved.
• Register persists before it acknowledges, and is idempotent on retry, so an acknowledged
  registration is always one the next leader will find.
• Startup re-grants leases to every executor that answers the initial health check, so a shard
  manager restart or failover never evicts a healthy cluster.
• etcd compaction. Every renewal is a new etcd revision, and etcd keeps them all until told
  otherwise. The leader compacts the history behind the state after each pass, keeping the newest
  compaction_retention_revisions (1000 by default; 0 disables it, for a cluster shared with other
  applications). A failure to compact is logged and never delays a pass.
• The deployment guide describes the lease and the compaction setting.

Decisions the ticket does not make

• No timer knob. The tick is derived from shard_lease_duration; a second setting would have to
  be kept consistent with it by hand.
• Renewal asserts epochs rather than advancing them. The ticket advances them on every renewal.
  That is not idempotent: one lost response leaves the executor an epoch behind for shards it still
  legitimately owns.
• Startup re-grants. Persisted expiries are absolute, so after any outage every one of them is in
  the past, and the ticket's "call housekeep at the top of each pass" would evict the whole cluster
  on pass one.
• No-op writes are skipped, except the first write on an empty store: the shard-count check on a
  replica needs stored state to compare against, and a cold leader would otherwise persist nothing
  until its first executor registered.
• New field numbers, and field 1 reserved. The ticket reuses shard_epochs = 1, which is a wire
  break against the shard_ids that lives there. The old-bytes decode test passes with or without the
  reserved lines, because prost skips unknown fields; what reserved buys is protoc's build-time
  refusal of a future re-use, so please don't simplify it away.
• ShardLeaseError is a oneof, mirroring QuotaError, rather than the error code the ticket
  names: ShardManagerError has nothing to attach a code to.
• A mismatched claim is corrected, not refused. The ticket refuses it with stale_epoch, so that a
  wrong picture is never extended. But refusing it holds an executor whose pushes cannot reach it - a
  one-way partition - on that wrong picture until its lease lapses and its agents restart, through the
  one channel that still works. Correcting the picture in the same round trip that extends the lease
  removes the objection. That is only safe with the revision order and the single receipt path above,
  which is why they exist; together they are push-plus-periodic-resync with a resource version, the
  shape informers use.
• Reaping lapsed leases is its own write, ahead of the renewal. The one refusal left, an unknown
  executor, discards its write whole, and must not take another executor's reaping with it.
• A failed delivery is repaired forwards, not rolled back. The store already holds the new
  ownership by the time anything is sent, so an executor that missed a revoke or a push is queued for
  a full push of the set it is now recorded as holding, rather than the plan being unwound. Its own
  next renewal carries the same set, so the repair is bounded even if the push fails again. This
  replaces stripping the failed shards out of the plan, which could re-mint an epoch that had already
  been pushed - two holders of one shard at the same epoch, the pair an oplog fence cannot separate.
• The assignment-changed hook is held weakly by the shard manager service, following
  LazyWorkerActivator: it closes over the service graph that owns that service, so a strong
  reference would be a cycle nothing could free. WorkerExecutorImpl owns it.
• The renewal loop races the RPC against the shutdown token, not just the sleep before it, and
  the renewal and deregister calls take one attempt rather than the client's retries: the loop owns
  the backoff, and a termination signal has to be seen inside the grace main waits.
• SIGTERM is handled in the executor's main. It trips the graph-wide shutdown token and waits a
  bounded grace on a TaskTracker that the renewal loop spawns through, so the deregister RPC is not
  cut off by runtime teardown. That is tokio-util's rt feature, declared on the workspace's existing
  dependency; create_shard_manager_service takes the whole Shutdown rather than its token.
• The renewal loop's Weak<Self> is not its exit; the shutdown token is. The assignment-changed
  hook makes the service reach itself, so upgrade() cannot fail. The cycle is the same shape as the
  pre-existing lazy_worker_activator one.
• Accepted: write volume. Persisted expiry means every renewal is a full-blob compare-and-swap
  write, about 3N per lease period, each a new etcd revision. The leader compacts the history
  behind them, so the volume costs etcd throughput but not disk; ticket 7 stacks quota renewals on
  top.

[netlify]

✅ Deploy Preview for golemcloud canceled.

Name	Link
🔨 Latest commit	e3c34e3
🔍 Latest deploy log	https://app.netlify.com/projects/golemcloud/deploys/6aa7f652014c550008619c3b

[vigoo]

Agent review on behalf of @vigoo — delayed TTL can extend admission past the manager’s lease expiry

lease_expiry_from_ttl adds the transmitted remaining TTL to the executor’s clock at receipt, without accounting for delivery time. For example, the manager has an expiry of t=60 and sends an AssignShards push at t=20 carrying TTL=40. If it arrives at t=25, the executor installs an expiry of t=65. If subsequent renewals fail and no newer delivery reaches that executor, a manager pass can reclaim and reassign the shards during t=60–65 while the old executor still admits work. This requires no clock skew, and revision ordering does not help when the delayed delivery is the newest one received.

Relevant paths: golem-common/src/model/protobuf.rs::lease_expiry_from_ttl, golem-worker-executor/src/grpc/mod.rs::assign_shards_internal, and the registration/renewal response decoding in golem-service-base/src/clients/shard_manager.rs.

For registration and renewal, a conservative approach is to anchor the returned TTL to a monotonic timestamp captured before sending the request, rather than to receipt time. Unsolicited pushes need a separate rule—for example, update the shard set/revision without extending lease validity, and establish or extend validity through a request-bound registration/renewal. This is a design direction, not a validated drop-in patch. Sending an absolute expiry instead only solves the issue if clock uncertainty is bounded and accounted for conservatively.

Suggested regression coverage: delay a still-newest delivery and verify that the executor’s admission deadline cannot outlive the manager’s safe reclamation boundary; cover both request/response grants and pushes. This finding is based on code inspection, not an executed distributed-system reproducer.

This concerns the admission fence implemented here, not the explicitly deferred oplog write fence. The previously discussed recovery-blocks-renewal finding has been withdrawn and is not an outstanding review finding.

[Aditya1404Sal]

What was wrong. The manager encoded lease_ttl at send and the executor decoded it as
Utc::now() + ttl at receipt, so every hop of delay landed on the executor's expiry and on nothing
else. Your t=20/t=25 example is exactly right, and the revision order cannot help, because the
delayed delivery is the newest one. It needed no skew.

The rules now.

1. A lease is anchored where it was asked for. The executor reads Instant::now() inside the
   innermost per-attempt closure of Register and RenewShardLease, before the request is first
   polled, and the expiry is sent_at + ttl. Not before GrpcClient::call: its internal
   Unavailable retries times the connect timeout can age a pre-call anchor by tens of seconds,
   which at the 30s minimum lease would arrive already lapsed. Since the manager grants after that
   instant, the executor's copy is short by the request's transit plus the manager's write time.
2. Only a grant moves the lease clock, and every grant moves it. Your separate rule for pushes,
   made structural: AssignShardsRequest.lease_ttl is gone from the proto, so an unsolicited push
   cannot extend validity even by accident - it carries the set and the revision only.
   RevokeShards never carried one. The other half follows from that: a renewal reply whose set
   is stale against a push that overtook it still moves the lease clock, because it answers the
   executor's own request and was anchored before that request left. Without it, every rebalance
   that crossed a reply would cost the executor a renewal, and an executor the manager keeps
   renewing could fence itself. The revision orders sets; the request instant orders leases. The
   write is a set, never a max, so a shard_lease_duration reduced across a restart shortens
   the executor's copy at its next renewal.
3. The executor's lease clock is std::time::Instant, per your suggestion. The manager keeps
   DateTime<Utc> because it persists expiries and re-reads them after a failover.

Two consequences worth flagging, both handled. With pushes carrying no lease, the first push
after a manager outage lands on an executor whose own copy has lapsed. It sweeps but does not
recover agents - a fenced executor starts nothing - and the grant that revives the lease runs the
recovery through the existing latch. So admission resumes on the next successful renewal rather
than on that push: at most the retry cap plus one call deadline, about 30s at the 1m default. And
a startup re-grant is now never shorter than the length the executor was last told, or a reduced
shard_lease_duration across a restart would lapse on the manager while the executor still held
the old length.

Coverage, as you asked, at both the grant and push paths: a property test drives both wire
functions over a grid of request transits and manager write times and asserts the executor's hold
never exceeds the manager's; one delays the answer and pins the expiry to send-plus-TTL exactly;
one pins that the send instant precedes the request rather than following its answer; one applies a
push and a revoke after a grant and asserts the lease clock did not move; one pins that a stale
grant keeps the pushed set and still moves the clock. Each was watched failing before the code
existed, and each is backed by a mutation that it catches.

One open question for you, because it is a policy number rather than a protocol property.

Anchoring removes every delay term but not rate error: both sides measure the lease on their
own clock, so an executor whose clock runs slow relative to the manager's outlives the manager's
expiry by that fraction of the lease, and a forward step of the manager's wall clock does the same.
Steady ntp/chrony discipline bounds it at roughly 500 ppm, about 30 ms on the 1m default; a
daemon slewing off a large offset does not, since chrony's default ceiling is a twelfth, about 5s on
a 1m lease, and CLOCK_MONOTONIC is slewed (only _RAW is not). I have stated it as an assumption
in the decoder's doc rather than invent an allowance.

If you would rather it were absorbed, the shape is the executor holding ttl * (1 - a), with a
alongside the other constants in base_model/shard_lease.rs and the derived minimums recomputed on
the shortened lease. Worth knowing before picking a number: the shipped shard_lease_duration
default of 1m is exactly the derived recommended minimum, so any a at all drops the effective
lease below it and the default starts warning at startup unless the default moves up with it. That
makes the real cost of an allowance the config change rather than the lease time - 1% would want a
61s default, a twelfth would want 66s, and a longer lease is a slower re-home after a crash.

[vigoo]

Agent review — outstanding lease deadlines must also be preserved on renewal. This protects against shortening during startup, but the first renewal calls renew_lease, which unconditionally replaces the deadline with now + lease_ttl.

Example: the executor has an acknowledged deadline of t=60. The manager restarts at t=15 with the duration reduced from 60s to 30s. A renewal at t=20 stores t=50; if its reply is lost and subsequent renewals fail, the executor retains t=60 while the manager can reclaim and redistribute its shards from t=50.

Please preserve the outstanding manager deadline on renewal, e.g. max(existing_expiry, now + lease_ttl), and add coverage for an early renewal whose response is lost. Updating the executor’s deadline on receipt cannot fix a response it never receives.

[vigoo]

Agent review — the initial startup caller needs to retain this deferred-recovery outcome. WorkerExecutorImpl::new currently discards the successful RecoveryOutcome. If registration returns a nonempty assignment with an already-expired request-anchored deadline, startup skips recovery without setting recovery_pending. A subsequent unchanged renewal restores admission but does not start those agents.

The queued repair push is not a guaranteed fallback: if that renewal advances the executor’s revision first, the older push is acknowledged as stale without performing recovery, ending the manager’s retries.

Please mirror the push path at startup: mark recovery pending before awaiting assignment effects, and clear it only on Recovered. Keep interruption ordering and startup-fatal errors unchanged. Add startup-path coverage for an expired nonempty registration followed by an unchanged live renewal.