rdma: track terminal events held off by an active reservation

A teardown callback that arrived while its record was reserved
was dropped: if the READY that held the reservation then rolled
its claim back, the session had lost both publication paths - no
request owner and no callback owner - and the record stayed
forever, unpublished. The record now stashes the terminal event;
releasing the reservation after a rollback consumes the stash and
publishes the expiry, since the native session is gone and no
second callback will arrive.

A duplicate READY that lost the reservation race still proceeded
to claim the native transfer, so the transfer could complete with
no publication owner. The READY handler now refuses to claim when
the reservation is not granted, answering as a duplicate claim.
Publication also validates ownership against the record's emitter,
so a stale reservation cannot publish over or consume the current
owner's record.

The shutdown drain raced its producers: a callback could enqueue
a publication after the drain checked the queue but before the
worker exited, stranding the record behind a stopped worker. The
drain now runs after the RC service close, which quiesces the
native reaper before returning, so no producer can enqueue behind
the drain; ordering replaces locking.

Error paths that published directly bypassed the single-shot
guard, letting the deferred panic safety net attempt a second
publication of the same record. All outcome publications in the
READY completion flow now go through the guarded path.
This commit is contained in:
Jihyeon Gim
2026-09-09 13:16:52 +09:00
parent 96be4d1407
commit d3e45fdb41
4 changed files with 95 additions and 13 deletions
+7 -5
View File
@@ -156,16 +156,18 @@ func (b *BackendShutdownAfterRC) SetOpsDrainer(d OpsDrainer) {
b.ops.Store(&d)
}
// Shutdown drains operational publications, closes the RC service,
// then shuts the wrapped backend down, once. The publication drain
// runs before the RC close: RC teardown itself queues publications,
// so the queue must still be moving while the sessions drain.
// Shutdown closes the RC service, drains operational publications,
// then shuts the wrapped backend down, once. The drain runs AFTER
// the RC close: closing RC quiesces the native reaper (every
// teardown callback has returned by the time Close returns), so no
// producer can enqueue behind the drain - enqueue-then-worker-exit
// stranding is impossible by ordering rather than by locking.
func (b *BackendShutdownAfterRC) Shutdown() {
b.closed.Do(func() {
b.rc.Close()
if d := b.ops.Load(); d != nil {
(*d).Shutdown()
}
b.rc.Close()
b.Backend.Shutdown()
})
}
+37 -3
View File
@@ -214,6 +214,11 @@ type sessionRecord struct {
// exactly once itself), so a completion call that fires the
// callback before returning cannot publish a placeholder.
reserved bool
// terminal marks a teardown that arrived while the record was
// reserved: the native session is gone and no second callback
// will come, so a later release of the reservation resolves
// the stashed outcome instead of leaving an orphan.
terminal bool
}
// opsTracker owns terminal publication: each session publishes
@@ -452,16 +457,28 @@ func (t *opsTracker) reserve(sessionID string) *opsEmitter {
// without publishing: the transfer claim it was held for rolled
// back, so the session lives on and the next claimant (another
// READY, or the reaper) must still find an unreserved record.
// If the native session already tore down while the record was
// reserved (terminal stashed), the session is gone: consume the
// record and publish the stashed outcome, since no second
// callback will arrive.
func (t *opsTracker) releaseReservation(sessionID string, emit *opsEmitter) {
if t == nil {
return
}
t.mu.Lock()
rec, ok := t.sessions[sessionID]
if ok && rec.reserved && rec.emit == emit {
rec.reserved = false
if !ok || !rec.reserved || rec.emit != emit {
t.mu.Unlock()
return
}
if !rec.terminal {
rec.reserved = false
t.mu.Unlock()
return
}
delete(t.sessions, sessionID)
t.mu.Unlock()
t.dispatch(pubJob{emit: rec.emit, err: rec.out.err, byt: rec.out.byt})
}
// publishReserved publishes through a reserved record and drops it:
@@ -471,6 +488,15 @@ func (t *opsTracker) publishReserved(sessionID string, emit *opsEmitter, err err
return
}
t.mu.Lock()
rec, ok := t.sessions[sessionID]
// Ownership check: only the reservation holder publishes. A
// stale holder (its reservation was released or the record
// was replaced) must not delete or publish the current
// owner's record.
if !ok || rec.emit != emit {
t.mu.Unlock()
return
}
delete(t.sessions, sessionID)
t.mu.Unlock()
if emit != nil {
@@ -502,8 +528,16 @@ func (t *opsTracker) onTerminal(ev rcserver.TerminalEvent) {
// A reserved record belongs to its request path, which
// publishes the real outcome itself: the callback (fired
// synchronously by a completion call, before the request
// path could confirm the result) must not touch it.
// path could confirm the result) must not touch it. But the
// terminal is still a fact: if the reservation is released
// later (claim rollback) and no second callback will ever
// come - the native session is gone - the stashed event
// resolves then, instead of being lost.
if rec.reserved {
if !rec.out.done {
rec.out = sessionOutcome{err: expiredError(ev), done: true}
}
rec.terminal = true
t.mu.Unlock()
return
}
+39 -2
View File
@@ -274,6 +274,43 @@ func TestOpsTrackerPublishesExactlyOncePerSession(t *testing.T) {
tr.releaseReservation("s-rel", emit3)
tr.onTerminal(rcserver.TerminalEvent{SessionID: "s-rel"})
// M1 regression: a terminal arriving while reserved is stashed,
// and a later claim-rollback release consumes it and publishes the
// expiry - the record is not orphaned.
tr.register("s-stash", auth.Account{Access: "ak"}, "r", "b", "o", false, time.Now())
emitS := tr.reserve("s-stash")
if emitS == nil {
t.Fatal("reserve failed")
}
tr.onTerminal(rcserver.TerminalEvent{SessionID: "s-stash"})
tr.releaseReservation("s-stash", emitS)
// M2 regression: a second reservation of a live record is
// refused, so a duplicate READY cannot claim the transfer
// while another request owns the publication. The owner then
// completes normally.
tr.register("s-dbl", auth.Account{Access: "ak"}, "r", "b", "o", false, time.Now())
emitD := tr.reserve("s-dbl")
if emitD == nil {
t.Fatal("first reserve failed")
}
if tr.reserve("s-dbl") != nil {
t.Fatal("double reserve succeeded")
}
tr.publishReserved("s-dbl", emitD, nil, 64)
// Ownership: a stale emitter must not publish or consume the
// current record; the real owner still can, even after the
// callback fired (stashed) underneath it.
tr.register("s-own", auth.Account{Access: "ak"}, "r", "b", "o", false, time.Now())
emitO := tr.reserve("s-own")
if emitO == nil {
t.Fatal("reserve failed")
}
tr.publishReserved("s-own", &opsEmitter{}, nil, 999)
tr.onTerminal(rcserver.TerminalEvent{SessionID: "s-own"})
tr.publishReserved("s-own", emitO, nil, 32)
// Consume-or-noop denial of an unreserved session.
tr.register("s-fail", auth.Account{Access: "ak"}, "r", "b", "o", false, time.Now())
tr.failOutcome("s-fail", errors.New("x"))
@@ -284,8 +321,8 @@ func TestOpsTrackerPublishesExactlyOncePerSession(t *testing.T) {
rl.mu.Lock()
defer rl.mu.Unlock()
if len(rl.logs) != 5 {
t.Fatalf("published %d records, want 5: %+v", len(rl.logs), rl.logs)
if len(rl.logs) != 8 {
t.Fatalf("published %d records, want 8: %+v", len(rl.logs), rl.logs)
}
// The recording sink cannot see session IDs directly (they
// live in the synthesized context), so assert the observable
+12 -3
View File
@@ -401,6 +401,15 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
// the record in the window between the claim and the
// reservation. A reserved record is invisible to both.
emit := h.ops.reserve(sessionID)
if emit == nil {
// Another READY holds the publication reservation for
// this session. Publication ownership must track native
// transfer ownership: proceeding without the reservation
// would let this request win the native claim while a
// different request owns the publication, losing the
// record on completion. Answer as a duplicate claim.
return fmt.Errorf("transfer in progress: %w", rcserver.ErrDouble)
}
publish := func(err error, bytes int64) {
h.ops.publishReserved(sessionID, emit, err, bytes)
}
@@ -494,7 +503,7 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
finalized = true
}
if err != nil {
publish(mapRcError(err), 0)
doPublish(mapRcError(err), 0)
return err
}
// The FINAL wire reply carries the stored object's
@@ -505,7 +514,7 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
// the same commit metadata.
emit.setCommitMeta(put.ETag, put.VersionID)
} else if err := h.svc.FinishFinal(sessionID); err != nil {
publish(mapRcError(err), 0)
doPublish(mapRcError(err), 0)
return mapRcError(err)
} else {
finalized = true
@@ -513,7 +522,7 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error {
// The transfer completed: publish the terminal record with
// the byte count the data plane reported.
publish(nil, int64(resp.BytesTransferred))
doPublish(nil, int64(resp.BytesTransferred))
// Wire reply per the hipobj-rc-v2 contract: protocol echo,
// cookie echo, transferred bytes, and object metadata.