diff --git a/rdma/rcroutes/ops_linux.go b/rdma/rcroutes/ops_linux.go index e055aa29..096b34a7 100644 --- a/rdma/rcroutes/ops_linux.go +++ b/rdma/rcroutes/ops_linux.go @@ -19,6 +19,7 @@ package rcroutes import ( "errors" + "net/http" "strings" "sync" "time" @@ -64,9 +65,10 @@ type opsEmitter struct { // synthesize builds a fiber context whose path and request locals // describe the session's logical object operation, so the standard // access-log and event pipelines observe GET/PUT of bucket/key -// instead of the fixed RDMA control path. The path string is the -// emitter's own storage: event senders serialize asynchronously, so -// the synthesized context must never hand them pooled buffers. +// instead of the fixed RDMA control path. The app runs with +// Immutable: string accessors copy instead of exposing the pooled +// context buffer, which matters because event senders serialize +// asynchronously and would otherwise read a reused buffer. func (e *opsEmitter) synthesize() (fiber.Ctx, func()) { ctx := e.app.AcquireCtx(&fasthttp.RequestCtx{}) method := fiber.MethodGet @@ -76,9 +78,7 @@ func (e *opsEmitter) synthesize() (fiber.Ctx, func()) { ctx.Method(method) // The access logger and the event schema both split this path // into bucket/key, so the synthesized path must be the object - // path in canonical form. fiber copies override strings it - // stores as the path original; the derived c.path below is - // a fresh allocation, which is what outlives the release. + // path in canonical form. ctx.Path("/" + e.bucket + "/" + e.key) utils.ContextKeyAccount.Set(ctx, e.acct) utils.ContextKeyRegion.Set(ctx, e.region) @@ -94,22 +94,15 @@ func (e *opsEmitter) synthesize() (fiber.Ctx, func()) { // committed PUT) the object-created event. Exactly-once delivery is // the tracker's job; this method just performs one emission. // -// The operational sinks classify plain errors as 500 on their own, -// which would disagree with the status the client saw. Before the -// record reaches them, the error is rendered as its mapped S3 error, -// so the audit log, the metric, and the wire response all carry the -// same classification. +// The operational sinks classify plain errors as 500 on their own +// and unwrap nothing, so the publication always hands them the +// error in its normalized S3 form: the audit log, the metric, and +// the wire response then carry the same classification. func (e *opsEmitter) publish(err error, bytes int64) { if e == nil || (e.ops.Logger == nil && e.ops.Metrics == nil && e.ops.Events == nil) { return } - sinkErr := err - if err != nil { - var s3Err s3err.S3Error - if !errors.As(err, &s3Err) { - sinkErr = routeError(err) - } - } + sinkErr := normalizeSinkError(err) ctx, release := e.synthesize() defer release() @@ -117,7 +110,10 @@ func (e *opsEmitter) publish(err error, bytes int64) { if e.isPut { action = metrics.ActionPutObject } - status := httpStatusFromError(sinkErr) + status := http.StatusOK + if sinkErr != nil { + status = sinkErr.(s3err.APIError).HTTPStatusCode + } if e.ops.Metrics != nil { e.ops.Metrics.Send(ctx, sinkErr, action, bytes, status) @@ -137,9 +133,26 @@ func (e *opsEmitter) publish(err error, bytes int64) { } } +// normalizeSinkError renders any operation error as the plain +// s3err.APIError the sinks expect: wrapped S3 errors keep their +// payload (the audit loggers assert the S3Error interface directly +// and would misclassify a wrapper), and non-S3 errors map through +// the same route error mapping the wire response uses. +func normalizeSinkError(err error) error { + if err == nil { + return nil + } + var s3Err s3err.S3Error + if errors.As(err, &s3Err) { + return s3Err.BaseError() + } + return routeError(err) +} + // httpStatusFromError maps an operation error to the HTTP status -// the S3 surface would have answered with. The callers pass mapped -// S3 errors, so the status is simply the error's own. +// the S3 surface would have answered with, using the same route +// error mapping as the wire response so operational records never +// disagree with what the client saw. func httpStatusFromError(err error) int { if err == nil { return 200 @@ -147,36 +160,42 @@ func httpStatusFromError(err error) int { return routeError(err).HTTPStatusCode } +// sessionOutcome is the terminal outcome of a tracked session, +// recorded by whichever path observes the final result first. A +// recorded outcome is final: the teardown callback consumes it and +// publishes, so exactly one publication happens per session. +type sessionOutcome struct { + err error // nil on success + byt int64 // bytes transferred on success + done bool // outcome recorded +} + // sessionRecord is one tracked session with its captured context. type sessionRecord struct { emit *opsEmitter - // reserved marks a record the request path holds exclusively: - // it took ownership before invoking a native completion call - // that would fire the teardown callback synchronously, so the - // callback must not publish on its behalf. - reserved bool + out sessionOutcome } -// opsTracker owns terminal publication: each session (and each -// pre-session request) is published exactly once, by whichever path -// confirms the final outcome first. It carries its own throwaway -// fiber.App for synthesizing publication contexts, independent of -// the gateway's request routing. +// opsTracker owns terminal publication: each session publishes +// exactly once. The request paths only ever RECORD an outcome; the +// native teardown callback - which the ABI guarantees fires exactly +// once per destroyed session, after every completion call - is the +// single publisher. This removes every ownership race: a recorded +// outcome cannot be double-published, and a record the callback +// already consumed cannot be resurrected. type opsTracker struct { mu sync.Mutex ops OpsServices sessions map[string]*sessionRecord - // earlyTerminals parks teardown notifications that arrived - // before the session's registration; register consumes them. - earlyTerminals map[string]rcserver.TerminalEvent - app *fiber.App + app *fiber.App } func newOpsTracker() *opsTracker { return &opsTracker{ - sessions: map[string]*sessionRecord{}, - earlyTerminals: map[string]rcserver.TerminalEvent{}, - app: fiber.New(), + sessions: map[string]*sessionRecord{}, + app: fiber.New(fiber.Config{ + Immutable: true, + }), } } @@ -192,23 +211,22 @@ func (t *opsTracker) SetOpsServices(ops OpsServices) { t.ops = ops } -// register captures the operational context of a successfully created -// session so a later terminal path can publish its final outcome. +// register captures the operational context of a successfully +// created session so the terminal outcome can be published later. // The strings are cloned: they originate from the request's pooled // header buffer, which does not survive the response. // -// handleEarlyTerminal covers the FinishPrepare race: the native call -// that finalizes PREPARE can reap an already-expired session and fire -// the teardown callback before register runs. When the callback wins -// that race it parks the event, and register consumes it instead of -// leaving an entry whose only notification already happened. +// The account is captured by value but its string fields still +// reference request storage on some IAM paths, so the sink-relevant +// identity is cloned as well. func (t *opsTracker) register(sessionID string, acct auth.Account, region, bucket, key string, isPut bool, start time.Time) { + acct.Access = strings.Clone(acct.Access) emit := &opsEmitter{ ops: t.loadOps(), app: t.app, acct: acct, - region: region, + region: strings.Clone(region), bucket: strings.Clone(bucket), key: strings.Clone(key), isPut: isPut, @@ -217,21 +235,14 @@ func (t *opsTracker) register(sessionID string, acct auth.Account, t.mu.Lock() defer t.mu.Unlock() - // A teardown notification that arrived before this registration - // owns the publication: publish now and store nothing. - if _, parked := t.earlyTerminals[sessionID]; parked { - delete(t.earlyTerminals, sessionID) - go emit.publish(errSessionExpired, 0) - return - } t.sessions[sessionID] = &sessionRecord{emit: emit} } // unregister drops a session entry whose PREPARE finalization -// failed: the native side is gone, so the teardown callback has -// either already published or will find nothing. A parked early -// notification is dropped with it (the failure publication covers -// the outcome). +// failed before the session was committed: the native side either +// rejected it (no callback will come) or already reaped it (the +// callback found no record and published nothing). The failure +// itself is published as a request record by the caller. func (t *opsTracker) unregister(sessionID string) { if t == nil { return @@ -239,45 +250,46 @@ func (t *opsTracker) unregister(sessionID string) { t.mu.Lock() defer t.mu.Unlock() delete(t.sessions, sessionID) - delete(t.earlyTerminals, sessionID) } -// reserve takes exclusive ownership of a session's publication -// before the request path invokes a native completion call -// (FinishFinal, FinishPut, or a reap-triggering mutation). Those -// calls fire the teardown callback synchronously while the session -// record is still live; reserving first keeps the callback from -// publishing an expiry record for a transfer that is completing -// right now. -func (t *opsTracker) reserve(sessionID string) *opsEmitter { - t.mu.Lock() - defer t.mu.Unlock() - rec, ok := t.sessions[sessionID] - if !ok || rec.reserved { - return nil - } - rec.reserved = true - return rec.emit -} - -// unreserve restores callback ownership when a reserved completion -// call did not after all retire the session (the caller failed -// before any state change). The record goes back to normal tracking -// unless a teardown notification landed meanwhile. -func (t *opsTracker) unreserve(sessionID string, emit *opsEmitter) { - if t == nil || emit == nil { +// failOutcome publishes a failed finalization exactly once: when +// the finalizing call already reaped the session its callback +// published (the entry is gone, this is a no-op); when no callback +// will ever come (the native side rejected the call) the entry is +// consumed and published here. +func (t *opsTracker) failOutcome(sessionID string, err error) { + if t == nil { return } t.mu.Lock() - defer t.mu.Unlock() rec, ok := t.sessions[sessionID] + if ok { + delete(t.sessions, sessionID) + } + t.mu.Unlock() if !ok { - // The session is gone: the completion call retired it and - // the reserved emitter is the only remaining owner, so - // nothing to restore. return } - rec.reserved = false + rec.emit.publish(err, 0) +} + +// recordOutcome stores the terminal outcome of a completing +// transfer. It never publishes: publication belongs to the teardown +// callback, which fires exactly once per session after the +// completion call returns. Recording is idempotent - the first +// outcome wins - so a panic-unwind path that records after the +// normal path changed nothing. +func (t *opsTracker) recordOutcome(sessionID string, err error, bytes int64) { + if t == nil { + return + } + t.mu.Lock() + defer t.mu.Unlock() + rec, ok := t.sessions[sessionID] + if !ok || rec.out.done { + return + } + rec.out = sessionOutcome{err: err, byt: bytes, done: true} } func (t *opsTracker) loadOps() OpsServices { @@ -286,65 +298,48 @@ func (t *opsTracker) loadOps() OpsServices { return t.ops } -// claim removes the session from the table and returns its emitter -// to exactly one publisher. A reserved record is only claimable by -// its reserving request path (the callback skips it). -func (t *opsTracker) claim(sessionID string, byRequest bool) *opsEmitter { - t.mu.Lock() - defer t.mu.Unlock() - rec, ok := t.sessions[sessionID] - if !ok { - return nil - } - if rec.reserved && !byRequest { - return nil - } - delete(t.sessions, sessionID) - return rec.emit -} - -// onTerminal is the native teardown callback path: publish sessions -// that no handler ever claimed (expired, abandoned, or canceled -// without a READY). +// onTerminal is the native teardown callback: the single publisher +// of session records. It consumes the recorded outcome (success, +// failure, or expiry when no outcome was ever recorded) and removes +// the entry, so exactly one publication happens per session no +// matter which path confirmed the result. func (t *opsTracker) onTerminal(ev rcserver.TerminalEvent) { if t == nil { return } - emit := t.claim(ev.SessionID, false) - if emit != nil { - // An expired or abandoned session never reached a final - // object result; the bytes staged for it did not become - // a transfer. - emit.publish(errSessionExpired, 0) + t.mu.Lock() + rec, ok := t.sessions[ev.SessionID] + if !ok { + t.mu.Unlock() return } - // A session tearing down before its registration ran: park the - // event so register can publish instead of orphaning a record - // whose only notification already happened. - t.mu.Lock() - if _, live := t.sessions[ev.SessionID]; !live { - t.earlyTerminals[ev.SessionID] = ev - } + delete(t.sessions, ev.SessionID) t.mu.Unlock() + + if !rec.out.done { + // No request path ever confirmed a result: the session + // expired, was abandoned, or was canceled. The event's + // outcome carries the native reason. + rec.out = sessionOutcome{err: expiredError(ev), done: true} + } + emit := rec.emit + emit.publish(rec.out.err, rec.out.byt) } -// publishClaimed publishes the terminal record from the request -// path (READY completion or failure). A nil tracker (no operational -// services) and an unknown session (already claimed or never -// registered) are both silent no-ops. -func (t *opsTracker) publishClaimed(sessionID string, err error, bytes ...int64) { - if t == nil { - return +// expiredError renders an unclaimed teardown as the error the +// publication carries, derived from the native outcome so the +// record names the real terminal reason instead of a generic one. +func expiredError(ev rcserver.TerminalEvent) error { + switch ev.Outcome { + case int(rcserver.ReadyWireFail): + return rcserver.ErrWire + case int(rcserver.ReadyTimeout): + return errTimeoutExpired + case int(rcserver.ReadyVerifyFail): + return errVerifyFailed + default: + return errSessionExpired } - emit := t.claim(sessionID, true) - if emit == nil { - return - } - var n int64 - if len(bytes) > 0 { - n = bytes[0] - } - emit.publish(err, n) } // publishRequest emits an operation record for a request that ended @@ -355,11 +350,12 @@ func (t *opsTracker) publishRequest(ctx fiber.Ctx, acct auth.Account, if t == nil { return } + acct.Access = strings.Clone(acct.Access) emit := &opsEmitter{ ops: t.loadOps(), app: t.app, acct: acct, - region: regionFromCtx(ctx), + region: strings.Clone(regionFromCtx(ctx)), bucket: strings.Clone(bucket), key: strings.Clone(key), isPut: isPut, @@ -377,10 +373,9 @@ func regionFromCtx(ctx fiber.Ctx) string { return "" } -// expiredOutcomeError renders a parked teardown notification as the -// error the publication carries: an internal S3 error whose code -// names the expiry, so the audit log keeps the descriptive code the -// plain error used to carry instead of the generic mapping. +// sessionExpiredError is the S3 error an expired or abandoned +// session publishes: an internal error whose code names the +// expiry, so the audit log keeps a descriptive code. type sessionExpiredError struct { s3err.APIError } @@ -390,3 +385,19 @@ var errSessionExpired = sessionExpiredError{APIError: s3err.APIError{ Description: "The RDMA transfer session expired before completion", HTTPStatusCode: 500, }} + +// errTimeoutExpired and errVerifyFailed classify unclaimed teardowns +// whose native outcome names a specific READY failure: the record +// then matches what the wire response for that failure carries. +var ( + errTimeoutExpired = sessionExpiredError{APIError: s3err.APIError{ + Code: "RdmaTransferTimeout", + Description: "The RDMA transfer timed out before completion", + HTTPStatusCode: 504, + }} + errVerifyFailed = sessionExpiredError{APIError: s3err.APIError{ + Code: "RdmaTransferVerifyFailed", + Description: "The RDMA transfer failed verification before completion", + HTTPStatusCode: 502, + }} +) diff --git a/rdma/rcroutes/ops_linux_test.go b/rdma/rcroutes/ops_linux_test.go index 4e4c3003..58e99f1c 100644 --- a/rdma/rcroutes/ops_linux_test.go +++ b/rdma/rcroutes/ops_linux_test.go @@ -34,12 +34,11 @@ import ( "github.com/versity/versitygw/s3err" ) -// The tests exercise the tracker's ownership semantics: the session -// table is the single source of truth for who may publish, so the -// assertions watch table membership rather than the emission itself -// (emission is a no-op without operational services wired in). +// The publication model: request paths only record outcomes; the +// teardown callback is the single publisher. The tests exercise +// the table semantics that guarantee exactly-once publication. -func TestOpsTrackerExpiryPublication(t *testing.T) { +func TestOpsTrackerCallbackPublishesExpiry(t *testing.T) { tr := newOpsTracker() tr.register("sess-1", auth.Account{Access: "ak"}, "us-east-1", "bkt", "obj", false, time.Now()) @@ -47,7 +46,8 @@ func TestOpsTrackerExpiryPublication(t *testing.T) { t.Fatalf("registered sessions = %d, want 1", got) } - // The reaper path claims and removes the session. + // The reaper path publishes for a session no READY ever + // recorded and removes the entry. tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-1"}) if got := len(tr.sessions); got != 0 { t.Fatalf("session survived terminal: %d", got) @@ -60,102 +60,92 @@ func TestOpsTrackerExpiryPublication(t *testing.T) { } } -func TestOpsTrackerRequestPathClaims(t *testing.T) { +func TestOpsTrackerRecordedOutcomeWins(t *testing.T) { tr := newOpsTracker() tr.register("sess-2", auth.Account{Access: "ak"}, "us-east-1", "bkt", "obj", true, time.Now()) - // READY completion claims the publication first. - tr.publishClaimed("sess-2", nil, 4096) - if got := len(tr.sessions); got != 0 { - t.Fatalf("request path left session: %d", got) - } - - // The late reaper callback finds nothing left. + // The READY path records the real outcome; the callback then + // consumes it instead of publishing an expiry. + tr.recordOutcome("sess-2", nil, 4096) tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-2"}) if got := len(tr.sessions); got != 0 { - t.Fatalf("reaper re-added session: %d", got) + t.Fatalf("session survived terminal: %d", got) } + + // A late second record is a no-op (entry gone). + tr.recordOutcome("sess-2", nil, 1) } -func TestOpsTrackerReserveBlocksCallback(t *testing.T) { +func TestOpsTrackerFirstRecordWins(t *testing.T) { tr := newOpsTracker() tr.register("sess-3", auth.Account{Access: "ak"}, "us-east-1", - "bkt", "obj", true, time.Now()) - - // The request path reserves before invoking a native - // completion call. - emit := tr.reserve("sess-3") - if emit == nil { - t.Fatal("reserve returned nil for a live session") - } - - // The synchronous teardown callback must not publish on a - // reserved record: the entry stays put. - tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-3"}) - if got := len(tr.sessions); got != 1 { - t.Fatalf("reserved session removed by callback: %d", got) - } - - // A double reserve is refused. - if tr.reserve("sess-3") != nil { - t.Fatal("double reserve succeeded") - } - - // The request path publishes through its reserved emitter. - tr.publishClaimed("sess-3", nil, 128) - if got := len(tr.sessions); got != 0 { - t.Fatalf("reserved session survived request publication: %d", got) - } -} - -func TestOpsTrackerEarlyTerminal(t *testing.T) { - tr := newOpsTracker() - - // Teardown notification for an unregistered session parks. - tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-4"}) - if got := len(tr.earlyTerminals); got != 1 { - t.Fatalf("early terminal not parked: %d", got) - } - - // Registration consumes it: no live entry remains, so no - // orphan record can outlive the session. - tr.register("sess-4", auth.Account{Access: "ak"}, "us-east-1", "bkt", "obj", false, time.Now()) - if got := len(tr.earlyTerminals); got != 0 { - t.Fatalf("early terminal not consumed: %d", got) - } - if got := len(tr.sessions); got != 0 { - t.Fatalf("orphan session entry created: %d", got) + + tr.recordOutcome("sess-3", nil, 100) + tr.recordOutcome("sess-3", errors.New("late"), 0) + + tr.mu.Lock() + rec := tr.sessions["sess-3"] + tr.mu.Unlock() + if rec == nil || rec.out.err != nil || rec.out.byt != 100 { + t.Fatalf("second record overwrote the first: %+v", rec.out) } } func TestOpsTrackerUnregister(t *testing.T) { tr := newOpsTracker() - tr.register("sess-5", auth.Account{Access: "ak"}, "us-east-1", + tr.register("sess-4", auth.Account{Access: "ak"}, "us-east-1", "bkt", "obj", false, time.Now()) - tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-5"}) - tr.register("sess-6", auth.Account{Access: "ak"}, "us-east-1", - "bkt", "obj", false, time.Now()) - tr.unregister("sess-6") + tr.unregister("sess-4") if got := len(tr.sessions); got != 0 { t.Fatalf("unregister left entries: %d", got) } + // The teardown callback for the unregistered session is a + // silent no-op (native side already rejected or reaped it). + tr.onTerminal(rcserver.TerminalEvent{SessionID: "sess-4"}) + if got := len(tr.sessions); got != 0 { + t.Fatalf("terminal resurrected entry: %d", got) + } } func TestOpsTrackerUnknownSession(t *testing.T) { tr := newOpsTracker() // Unknown sessions and the nil tracker are silent no-ops. var nilTracker *opsTracker - nilTracker.publishClaimed("ghost", nil, 1) + nilTracker.recordOutcome("ghost", nil, 1) nilTracker.onTerminal(rcserver.TerminalEvent{SessionID: "ghost"}) - tr.publishClaimed("ghost", nil, 1) + tr.recordOutcome("ghost", nil, 1) tr.onTerminal(rcserver.TerminalEvent{SessionID: "ghost"}) if got := len(tr.sessions); got != 0 { t.Fatalf("ghost session materialized: %d", got) } } +func TestNormalizeSinkError(t *testing.T) { + if normalizeSinkError(nil) != nil { + t.Fatal("nil error should stay nil") + } + // Plain errors map through the route error mapping. + got := normalizeSinkError(errors.New("x")) + apiErr, ok := got.(s3err.APIError) + if !ok || apiErr.HTTPStatusCode != 500 { + t.Fatalf("plain error => %#v", got) + } + // Wrapped S3 errors are extracted to their base form so the + // audit loggers' direct assertion classifies them correctly. + wrapped := errWrapped{s3err.GetAPIError(s3err.ErrNoSuchBucket)} + got = normalizeSinkError(wrapped) + apiErr, ok = got.(s3err.APIError) + if !ok || apiErr.Code != "NoSuchBucket" || apiErr.HTTPStatusCode != 404 { + t.Fatalf("wrapped error => %#v", got) + } +} + +type errWrapped struct{ s3err.S3Error } + +func (errWrapped) Error() string { return "wrapped" } + func TestHttpStatusFromError(t *testing.T) { if got := httpStatusFromError(nil); got != 200 { t.Fatalf("nil error => %d, want 200", got) @@ -172,3 +162,25 @@ func TestHttpStatusFromError(t *testing.T) { t.Fatalf("limit error => %d, want 429", got) } } + +func TestExpiredErrorClassification(t *testing.T) { + cases := []struct { + outcome int + code string + status int + }{ + {int(rcserver.ReadyWireFail), "RdmaTransferFailed", 502}, + {int(rcserver.ReadyVerifyFail), "RdmaTransferVerifyFailed", 502}, + {int(rcserver.ReadyTimeout), "RdmaTransferTimeout", 504}, + {int(rcserver.ReadyOK), "SessionExpired", 500}, + } + for _, tc := range cases { + err := expiredError(rcserver.TerminalEvent{Outcome: tc.outcome}) + apiErr := normalizeSinkError(err).(s3err.APIError) + if apiErr.Code != tc.code || apiErr.HTTPStatusCode != tc.status { + t.Fatalf("outcome %d => %s/%d, want %s/%d", + tc.outcome, apiErr.Code, apiErr.HTTPStatusCode, + tc.code, tc.status) + } + } +} diff --git a/rdma/rcroutes/routes_linux.go b/rdma/rcroutes/routes_linux.go index 06da7b93..580f8d32 100644 --- a/rdma/rcroutes/routes_linux.go +++ b/rdma/rcroutes/routes_linux.go @@ -42,6 +42,7 @@ import ( "github.com/versity/versitygw/backend" "github.com/versity/versitygw/rdma/rcserver" "github.com/versity/versitygw/s3api/utils" + "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3response" ) @@ -232,11 +233,13 @@ func (h *Handler) prepareCore(ctx fiber.Ctx) error { h.ops.register(resp.SessionID, acct, regionFromCtx(ctx), bucket, key, isPut, time.Now()) if err := h.svc.FinishPrepare(resp.SessionID, true); err != nil { - // The finalization failed: the session is gone and the - // teardown callback (or this call's own reap) published - // the terminal record; nothing may keep the entry. - h.ops.unregister(resp.SessionID) - h.ops.publishRequest(ctx, acct, mapRcError(err), bucket, key, isPut) + // The finalization failed. Exactly one publication + // covers it: the finalizing call already reaped the + // session and its callback published the recorded + // outcome, or the native side rejected the call and no + // callback is coming, in which case the entry is + // published here. + h.ops.failOutcome(resp.SessionID, mapRcError(err)) return mapRcError(err) } @@ -417,21 +420,27 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error { // this handler owns the completion ref. A panic or early // unwind must still release it so the session can be reaped. // - // The publication ownership moves here as well: every native - // completion call below (FinishFinal, FinishPut) fires the - // teardown callback synchronously, and a reserved record is - // invisible to that callback, so the outcome is published by - // this handler exactly once. - emit := h.ops.reserve(sessionID) - publish := func(err error, bytes int64) { - if emit != nil { - emit.publish(err, bytes) - emit = nil + // This handler only RECORDS the outcome; publication belongs + // to the teardown callback, which every native completion + // call below fires exactly once. The deferred recorder covers + // panic unwinds too, so every path through this handler + // leaves a final outcome behind. + recorded := false + record := func(err error, bytes int64) { + if !recorded { + recorded = true + h.ops.recordOutcome(sessionID, err, bytes) } } finalized := false defer func() { if !finalized { + // The unwind finalizer retires the session, which + // fires the callback; make sure it carries an + // outcome even on a panic path. + if !recorded { + record(errPanicked(), 0) + } _ = h.svc.FinishFinal(sessionID) } }() @@ -449,7 +458,7 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error { finalized = true } if err != nil { - publish(mapRcError(err), 0) + record(mapRcError(err), 0) return err } // The FINAL wire reply carries the stored object's @@ -457,15 +466,19 @@ func (h *Handler) readyCore(ctx fiber.Ctx) error { resp.Etag = put.ETag resp.VersionID = put.VersionID } else if err := h.svc.FinishFinal(sessionID); err != nil { - publish(mapRcError(err), 0) + record(mapRcError(err), 0) return mapRcError(err) } else { finalized = true } - // The transfer completed: publish the terminal record with - // the byte count the data plane reported. - publish(nil, int64(resp.BytesTransferred)) + // The transfer completed: record the terminal outcome with + // the byte count the data plane reported. FinishPut (inside + // commitPut) or the FinishFinal above already fired the + // teardown callback, which publishes this outcome - or, on a + // timing edge where the callback ran first, the deferred + // finalizer's FinishFinal does. + record(nil, int64(resp.BytesTransferred)) // Wire reply per the hipobj-rc-v2 contract: protocol echo, // cookie echo, transferred bytes, and object metadata. @@ -699,3 +712,14 @@ func mapRcError(err error) error { } return err } + +// errPanicked is the outcome recorded when a panic unwinds the +// READY handler after the completion ref was claimed: the record +// keeps the failure even though the panic itself propagates. +func errPanicked() error { + return s3err.APIError{ + Code: "InternalRDMAError", + Description: "The RDMA transfer ended without a confirmed result", + HTTPStatusCode: 500, + } +}