Skip to content

[Rust][Arrow] Move explicit close into supervisor - #680

Open
teodordelibasic-db wants to merge 5 commits into
mainfrom
effort/zerobus-sdk-supervisor-owned-close
Open

[Rust][Arrow] Move explicit close into supervisor#680
teodordelibasic-db wants to merge 5 commits into
mainfrom
effort/zerobus-sdk-supervisor-owned-close

Conversation

@teodordelibasic-db

@teodordelibasic-db teodordelibasic-db commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What changes are proposed in this pull request?

Arrow Flight close previously split responsibility between foreground flush, connection rotation, recovery, and teardown. That made the result depend on which path observed a close or transport failure first.

This change gives the background supervisor sole ownership of close and terminal finalization:

  • Add a private Open -> Requested -> Finalized close coordinator. The first close() snapshots its SDK-offset target and absolute deadline under the ingestion mutex, so repeated or cancelled calls await the same request and result.
  • Use one active/waiting/draining connection lifecycle for explicit close and server-requested rotation. An active connection waits for the close target or deadline, half-closes once, and drains request EOF plus the response status within a bounded interval. Close that interrupts an already-active rotation completes that drain and returns the rotation trigger even if its SDK-offset target is already durable.
  • Latch close-target completion at durable ACK application time. Supervisor scheduling cannot make a pre-deadline ACK late or a post-deadline ACK successful; late ACKs still reduce the retained suffix without replacing a selected timeout or concrete error.
  • Cancel recovery when close wins before replacement-sender publication, retain the exact unacknowledged suffix, and return the error that triggered that attempt even if the close target is already durable. Once a failed attempt is accepted for retry, its concrete error becomes the next attempt's trigger. If sender publication wins under the ingestion mutex, the replacement becomes active and receives normal graceful close.
  • Limit guaranteed graceful half-close and drain to the active connection. An established replacement whose sender has not committed is dropped best-effort when recovery is cancelled.
  • Serialize every replay handoff and replacement-sender publication with close without holding the ingestion mutex across channel-capacity waits.
  • Validate ACK, recovery, and flush deadlines against the platform monotonic-clock range.
  • Abnormal supervisor exit (abort or panic) no longer leaves close() waiting forever. A detached reaper finalizes the coordinator, preserves a returned error, and maps panic, cancel, or unexpected success to an invariant error while retaining the unacked suffix.
  • Terminal finalization closes admission before publishing is_closed, so a new ingest cannot return success in the snapshot window. close() re-reads coordinator state under the ingestion mutex so an unrepresentable flush deadline cannot mask a concurrently finalized peer error.
  • After a request-send failure, one ready response is processed first so a buffered ACK or terminal status is observed. If the send failure remains pending, the next loop forces recovery without polling another response, so a later PutResult is not discarded from the stream.

A close error during recovery or server-requested rotation means that attempt was interrupted. It does not mean the close target is undurable. Callers must inspect get_unacked_batches(); that set can be empty even when close() returns an error.

The change is internal to the Rust Arrow Flight implementation. It does not change public signatures, FFI signatures, ABI, or semver compatibility.

Fixes #657.

How is this tested?

  • ack_before_request_is_latched_timely, request_before_ack_is_latched_timely, ack_at_deadline_is_not_latched_timely, and preacked_close_preserves_latched_timeout cover both ACK/publication orderings and the exact flush-deadline boundary.
  • published_close_is_not_starved_by_ready_malformed_responses and ready_terminal_eof_precedes_published_empty_close cover response/close ordering without allowing a ready response stream to starve close.
  • close_during_rotation_ack_wait_preserves_rotation_state and test_late_drain_ack_does_not_replace_close_timeout cover rotation transitions, timeout selection, late ACK application, and rotation-trigger retention.
  • close_publication_precedes_queued_replay_handoff and published_close_prevents_sender_commit cover the ingestion-mutex ordering boundary for replay and sender publication.
  • test_close_during_reconnect_transport_handshake_preserves_trigger_and_suffix covers cancellation while a replacement proxy CONNECT handshake is pending. test_close_during_reconnect_setup_preserves_recovery_trigger covers cancellation while the replacement waits for READY.
  • test_close_after_partial_replay_preserves_trigger_and_suffix and test_close_during_recovery_backoff_preserves_trigger_and_suffix cover recovery cancellation and exact suffix retention.
  • test_close_during_second_recovery_attempt_preserves_latest_trigger covers close during attempt 2 returning attempt 1's reconnect error and the unacked suffix.
  • test_supervisor_recovery_after_retriable_error verifies that a committed replacement receives exactly one half-close and no reset.
  • test_unrepresentable_runtime_close_deadline_does_not_publish_close, test_unrepresentable_runtime_recovery_deadline_is_rejected, and test_close_during_recovery_backoff_preserves_trigger_and_suffix establish real Flight streams before pausing Tokio time. test_replay_ack_deadline_starts_after_replay_completes prevents implicit clock advancement while its real transports are active.
  • test_cancelled_close_rejects_ingest_and_resumes_teardown covers cancellation-safe repeated close. test_supervisor_abort_after_close_request_finalizes_exact_suffix covers abort after a published close request with bounded completion and the exact unacked suffix. non_finalized_supervisor_exit_is_an_invariant_error and reaper_preserves_worker_error_and_rejects_unfinalized_success cover defensive terminal-state mapping.
  • test_terminal_finalization_rejects_new_ingest_before_snapshot_publish and test_empty_flush_waits_for_terminal_outcome_during_finalization cover the admission-closed window before is_closed.
  • ready_ack_is_applied_before_reported_request_send_failure and ready_server_error_wins_reported_request_send_failure cover the one response-first tie. continuously_ready_nonprogress_responses_do_not_starve_send_failure covers infinite no-progress and malformed responses.

@teodordelibasic-db
teodordelibasic-db force-pushed the effort/zerobus-sdk-supervisor-owned-close branch 2 times, most recently from a4094a4 to f54f167 Compare August 6, 2026 14:33
@teodordelibasic-db teodordelibasic-db self-assigned this Aug 6, 2026
@teodordelibasic-db
teodordelibasic-db force-pushed the effort/zerobus-sdk-supervisor-owned-close branch 13 times, most recently from fcbf50c to 7381d13 Compare August 12, 2026 15:24
Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
@teodordelibasic-db
teodordelibasic-db force-pushed the effort/zerobus-sdk-supervisor-owned-close branch from 7381d13 to 6ca543b Compare August 12, 2026 19:25
Supervisor panic or abort left close() waiting on CloseState forever.
A detached reaper now finalizes the coordinator, and admission closes
before the unacked snapshot is published.

Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
Continuously ready no-progress responses could starve recovery after a
request send failure. One response-first tie is allowed; a buffered
terminal status or EOF still wins over the local Unavailable error.

Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
A second now_or_never poll consumed a ready PutResult without applying
it. Force send-failure on the next loop without polling another response.

Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
Signed-off-by: teodordelibasic-db <teodor.delibasic@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Rust][Arrow] Coordinate explicit close with recovery

1 participant