grpc: close the attach window where an executor negotiates no lease at all
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 20
- Forks
- 2
- Avg merge
- 5h 10m
- Merged PRs (30d)
- 127
Description
Follow-up to #932, which was fixed by its option 1. Option 2 — "install the lease book
before the door accepts a stream" — was left open because it is not available as written.
Surfaced by the Go executor e2e suite in #928.
What #932 fixed, and what it left
#932 made the check and the advertisement come off one decision: a capability is in force
only where the executor advertised it and that attach's hello_ack carried it. Frames
are no longer dropped, and the contract sentence is true as written.
What it does not do is make the window stop existing. An executor that attaches before this
process wins the scheduler role is acknowledged without lease, is dispatched without one,
and is fenced on (owner, attempt) alone for the life of that connection. That is exactly
the give-up CAP_LEASE
documents for a peer that does not negotiate the capability, and it is self-healing —
FLEXIQ_GRPC_EXECUTOR_STREAM_MAX_AGE rotates the stream (1800s by default) and the next
attach negotiates leases. So it is a real gap, not a bug.
The gap is requeue_stuck-shaped: while it is open, a dispatch of a job made under one
claim cannot be told from a later dispatch made under a new claim, so an operator
unsticking a job the executor is still running gets two attempts that can both report.
Why the ordering does not allow the obvious fix
The book belongs to the Scheduler. Scheduler::new creates it
(crates/flexiq-core/src/scheduler/mod.rs), worker/runner.rs hands it to the pool via
WorkerDispatcher::set_lease_book, and in flexiq-server the Scheduler only exists once
SchedulerSupervisor wins the role — long after listener::spawn and the gRPC service start
accepting (crates/flexiq-server/src/runtime/mod.rs). There is no point in the current
ordering at which the door could wait for a book.
Proposal: invert who owns the book
Have RemoteDispatcher::new mint a LeaseBook and always advertise CAP_LEASE, and have
the Scheduler adopt the dispatcher's book instead of creating its own. Nothing can be
dispatched before a scheduler exists, so a dispatcher that holds a book from construction is
not promising anything it cannot do.
Two things this buys:
- Every attach negotiates leases, whatever the election timing. The window closes rather
than being tolerated. - One book spans scheduler-role restarts. Today each generation builds a fresh book, so a
straggler from the previous generation meets an empty one and is accepted on the storage
fence alone. A book that outlives the generation keeps it fenced.
Places it touches:
crates/flexiq-core/src/scheduler/mod.rs—Schedulertakes a book rather than
constructing one.crates/flexiq-core/src/worker/mod.rs— theWorkerDispatchertrait, where
set_lease_bookbecomes an adopt-or-offer shape. The default no-op behaviour for pools
that do not fence (seecrates/flexiq/src/pool.rs) has to survive.crates/flexiq-core/src/worker/runner.rs— the wiring at theset_claim_owner/
set_lease_bookpair.crates/flexiq-server/src/runtime/mod.rsand the supervisor — the server's construction
order.crates/flexiq-core/src/worker/http_target/mod.rs— unaffected in behaviour (a push target
negotiates nothing), but it implements the same trait method.
Worth confirming while doing it: LeaseBook::retire/forget are already guarded against a
straggler evicting a newer dispatch's entry, which is what makes a longer-lived book safe.
Also left undone
#932's fix documented the rule in worker/protocol.rs and
REMOTE_SDK_CONTRACT.md,
but not in contracts/proto/flexiq/executor/v1/executor_service.proto. The descriptor
embeds proto comments, so a comment-only edit there makes contracts/descriptor.binpb stale
and needs scripts/proto-check.sh --fix with the pinned buf. Cheap to fold into any change
that already regenerates the descriptor — the two spots are HelloAckFrame.capabilities and
JobFrame.lease.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with RemoteDispatcher::new, Scheduler in crates/flexiq-core/src/scheduler/mod.rs, and the set_claim_owner/set_lease_book wiring in worker/runner.rs. Trace construction through crates/flexiq-server/src/runtime/mod.rs and preserve the no-op trait behavior in crates/flexiq/src/pool.rs and http_target/mod.rs. Done means every attach negotiates a lease, the book survives scheduler-role restarts, and the proto comments and descriptor remain consistent if updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- grpc, rust
- Domain
- api, backend, distributed-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100