Skip to content

magnetar 1.7.2

Choose a tag to compare

@FlorentinDUBOIS FlorentinDUBOIS released this 21 Sep 08:39
· 174 commits to main since this release

Fixed

  • Consumer::resubscribe() now closes the consumer before re-subscribing it, instead of handing the broker a second permit window it never agreed to.
    The in-place recovery re-emitted CommandSubscribe for the SAME, still-live consumer id, on the premise its own documentation stated: "the broker recreates its dispatcher slot at availablePermits = 0".
    Apache Pulsar does not.
    ServerCnx.handleSubscribe looks the consumer id up in its own per-connection consumers map before anything else, and a consumers.putIfAbsent hit on an already-completed consumer logs a warning, calls sendSuccessResponse(requestId), and returns — no new Consumer, no dispatcher or subscription touch, and nothing on that branch reads MESSAGE_PERMITS, availablePermits or flowPermits.
    Verified identical at v4.0.4 ServerCnx.java:1320-1326, v4.2.4 :1408-1414 and master :2037-2044; the sibling branches are deliberately not no-ops (a still-pending creation is answered ServiceNotReady, an exceptionally-completed one a mapped error), and neither SubType nor durability is read there at all.
    So the client zeroed its permit mirrors against a broker state that never happened, the broker kept its slot and its balance, and the re-subscribe Success then granted a full fresh receiver_queue_size window on top of what the broker already held — once per recovery attempt, while the wedge the call exists to clear could not move.
    Measured on the differential harness with a faithful live-id broker model: one recovery took an 8-message receiver queue's broker-observed balance from 8 to 16.
    Issue #414's production signature is a subscription permit counter at -177300, and a recovery that adds permits the broker never agreed to is moving that same counter, in the opposite direction, on the client's own initiative.
    The recovery is now two phases on the same live socket, with no transport reconnect: a CommandCloseConsumer for this consumer id, and then — on that close's Success only — the permit-mirror zeroing, the issue #346 in-flight-ack sweep, and the CommandSubscribe whose own Success releases the initial CommandFlow.
    Every mutation moved to the second phase, so a broker that rejects the close leaves the consumer byte-for-byte as the caller found it: still wedged, but still holding the positive permit_balance that is_stall_candidate requires, so it stays visible to the watchdog and eligible for another attempt.
    Zeroing up front and then failing to close would have produced a consumer that is wedged AND undetectable.
    Failover and non-durable subscriptions are now refused outright, returning Err without emitting anything: the close is real, so for a Failover consumer it triggers an election and a priority-order reshuffle to repair one slot, and for a non-durable one it discards the cursor that lives only as long as the consumer, making the re-attach restart from the configured start position (ADR-0099).
    Both refusals sit in resubscribe_consumer_in_place and not in the eligibility predicate shared with issue #307's broker-initiated CommandCloseConsumer arm, where the broker has already closed the consumer and refusing would only leave it detached forever.
    Three consequences are worth planning for.
    A recovery now costs one extra broker round trip before the grant re-arms.
    It also costs real redelivery: the broker genuinely drops the consumer, so everything it held un-acked returns to the subscription's redelivery pool, and acks the grouping tracker emits inside the close-to-re-subscribe window are dropped and their messages redelivered — at-least-once is preserved, duplicates are not.
    And consumer_stall_auto_recovery no longer pays down a dispatcher-wide leak: ADR-0103's "one attempt credits the aggregate back exactly receiver_queue_size" was true only because of the over-commit removed here, so one attempt is now permit-neutral on the subscription's aggregate and pulsar-admin topics unload remains the escalation for a negative counter.
    A connection lost between the two phases is won by the reconnect path — reset and fail_all_pending drop the pending close without leaking an outcome, and rebuild_consumers owns the re-attach — and a racing issue #307 broker close leaves exactly one CommandSubscribe, not two.
    (issue #414; ADR-0108, amending ADR-0101 §3's step list and its availablePermits = 0 premise and superseding ADR-0103's budget arithmetic)

  • A dropped ack() future no longer leaks a permanent outcomes entry — same issue #241 leak shape as the producer-close fix, now on the ack bookkeeping.
    Both runtime engines' RequestFut::drop clear the parked waker and take any outcome that already landed, but neither calls cancel_request: magnetar-runtime-moonpool/src/consumer.rs and magnetar-runtime-tokio/src/consumer.rs both stop at unregister_waker + take_outcome.
    So a caller that dropped its ack() future before it resolved — a select! timeout, a cancelled task — left the pending_requests entry behind with no waker, which is exactly the state three sweeps in crates/magnetar-proto/src/conn.rs then hit: fail_acks_orphaned_by_consumer_reattach (the same-broker CommandCloseConsumer sweep, issue #346), the ack_response_timeout backstop reap in handle_timeout, and the AckResponse command arm.
    All three recorded an OpOutcome in the connection's outcomes map unconditionally, mirroring neither the Success arm's unsubscribe_has_waiter guard nor its ProducerCloseForgotten / ConsumerCloseForgotten fire-and-forget handling, both added for issue #241's continuous-eviction leak.
    Nothing ever prunes outcomes wholesale — entries leave only via take_outcome or cancel_request — so every dropped ack whose resolution reached one of these three sites left one permanent entry.
    The two sweep sites now record an outcome only when self.wakers still holds a waker for the request, mirroring unsubscribe_has_waiter; the AckResponse arm now records one only when pending_requests.remove actually found an entry, mirroring the Success arm's None => {} case — a late broker reply for an ack an earlier sweep already resolved no longer overwrites outcomes with a second, undrainable entry.
    pending_requests.remove, the total_acks_failed accounting, and the emitted ConnectionEvent::AckResponse stay unconditional in all three sites; only the outcomes write and its paired wake are guarded.
    A caller that still holds its ack() future sees no change: the guard only ever skips a record nothing is left to consume.
    (issue #346-adjacent, same defect class as issue #241; not issue #414 — no resubscribe_consumer_in_place call is involved)

  • A dead-lettered dispatch unit now returns its flow permit, so a poison-heavy Shared subscription no longer wedges at availablePermits=0.
    The broker charges one permit per dispatch unit — Consumer#sendMessages debits ackedCount - totalMessages — and a unit the client will route to its dead-letter buffer is inside totalMessages, because the broker has no idea what the client intends to do with the entry.
    ConsumerState::classify_and_queue mirrored that debit correctly: record_dispatch_unit() runs unconditionally, before the queued-versus-dead-lettered branch.
    What it never did was hand the permit back.
    consumed_since_flow — the counter maybe_flow compares against max(receiver_queue_size / 2, 1) — was credited only by pop_message, the incomplete-chunk buffer, and the PIP-33 marker filter, and a dead-lettered unit is never queued, so it is never popped.
    No later path compensated either: drain_dead_letter is a std::mem::take, Connection::ack never touches the flow ledger, and republish_dead_letters is drain, send, ack.
    So each dead-lettered unit was a net -1 permit, drifting one way by exactly the dead-letter count until a churn boundary zeroed both mirrors.
    A receiver queue's worth of poison drove permit_balance to zero with consumed_since_flow still at zero, and nothing could recover it: maybe_flow was unreachable with nothing left to pop, is_stall_candidate requires permit_balance > 0 so the issue #414 watchdog and ADR-0103's automatic recovery never fired, and is_flow_starved is read only by the Failover promotion re-arm and Connection::initial_flow — churn and election boundaries a Shared subscription never reaches on its own.
    The broker showed availablePermits=0 and msgRateOut=0 with no error and no reconnect, and Consumer::available_permits() read 0.
    The dead-letter branch now calls record_broker_permit_consumed() before it buffers the message, which is the rule the other three refund sites already followed and the one Java follows in both dispatch shapes: ConsumerImpl.messageReceived calls increaseAvailablePermits(cnx) immediately after it skips an over-redelivered message, and receiveIndividualMessagesFromBatch accumulates skippedMessages and calls increaseAvailablePermits(cnx, skippedMessages) once per entry.
    There is no new emission site: conn.rs's Message arm already calls maybe_flow() after every deliver, so the inbound frame that crosses the half-queue threshold carries the CommandFlow out, batched entries whose members all dead-letter included.
    The invariant is now uniform — granted window == permit_balance + consumed_since_flow + queue.len() — with Delivered refunded at pop, Buffered refunded at buffering, Dropped moving neither side, and an ADR-0105 ack-set-cleared position moving neither side because the broker never charged it.
    Two consequences are worth planning for.
    The accidental backpressure is gone, so a poison-heavy topic keeps dispatching and dead_letter_pending — an unbounded Vec — grows until the application calls drain_dead_letter or republish_dead_letters; nothing auto-drains it.
    And permits ALONE no longer wedge the consumer: a dead-lettered unit stays unacked at the broker until republish_dead_letters acks it, so an application that never drains still stops at the broker's maxUnackedMessagesPerConsumer.
    On upgrade, a subscription that had parked at availablePermits=0 resumes draining and its DLQ topic may receive a burst.
    (issue #437; ADR-0107, amending ADR-0082's "pop-driven consumed_since_flow" parenthetical and ADR-0105's "consumed_since_flow only moves on pop_message" clause)

  • A Failover consumer whose permit balance was drained to zero without crossing the flow threshold is re-armed on promotion.
    ConsumerState::is_flow_starved is true when the broker has dispatched every granted permit and even popping the whole remaining queue could not push consumed_since_flow across the half-queue threshold — so maybe_flow is unreachable and both sides wait on each other forever.
    It is now read by both initial_flow gates: the issue #307 CommandActiveConsumerChange { is_active: true } promotion re-arm and Connection::initial_flow itself, whose additive granted_permits == 0 gate a previously-fed consumer never satisfies again outside a churn boundary.
    A fed consumer with permits still in flight gets no grant, so the issue #427 no-double-grant contract is unchanged.
    This was shipped before the issue #437 refund above and is retro-documented here; with the refund in place the predicate is no longer reachable through well-formed wire frames and remains as defence-in-depth for a broker-side debit the client mirror missed.
    (issue #443, PR #444; recorded in ADR-0107)

  • A broker CommandCloseProducer on a connection that stays up no longer leaves that producer detached until the process restarts.
    pulsar-admin topics unload of one partition of a partitioned topic makes the owning broker remove the producer and write the frame on a connection that keeps serving every other producer and consumer on it — no TCP drop, so no supervised reconnect and no rebuild_producers.
    The Type::CloseProducer arm only shut the send-drain gate and pushed a ProducerClosedByBroker event that nothing reads for an already-assembled producer, so the slot sat at closed = false, broker_ready = false, open_request_id = None for the life of the connection and every publish the round-robin router sent to that partition failed code=-1 send timeout forever.
    The producer is now re-attached in place on the same connection with a bumped epoch, exactly as the CommandCloseConsumer twin has done since issue #307, and the gate stays shut until the broker's fresh CommandProducerSuccess replays the staged publishes.
    A ServiceNotReady answer while the bundle reloads — the expected one, since the broker fences the topic before writing the close — rides the existing operation-retry leg (2 s initial backoff plus a lookup) and gives up with a terminal error after the configured budget instead of hanging.
    An assigned_broker_service_url on the close takes the same in-place path: no path owned that event shape on a live socket, and an Extensible-Load-Manager multi-phase unload makes it the default close shape.
    A warn! now records every such close with the handle, topic, request id, epoch and truncated assigned URL.
    ProducerClosedByBroker is surfaced only while a producer open is still in flight, where its parked waiter is the one reader that can consume it; a closed or unknown handle pushes nothing.
    (issue #451; ADR-0106)

  • magnetarctl no longer rejects the -1 sentinel when it is passed space-separated on the policy-setting commands.
    By default clap reads a token beginning with - as a flag, and none of these arguments carried one of the opt-ins that relax that, so --time-minutes -1 was read as an unknown short flag and magnetarctl admin namespaces set-retention acme/ns --time-minutes -1 --size-mb -1 failed with error: unexpected argument '-1' found — even though -1 is the documented "infinite / unlimited" value on every one of those flags.
    That error also carried clap's own tip: to pass '-1' as a value, use '-- -1', which is wrong here: -- ends option parsing rather than escaping the next token, so following the tip returned the same unexpected argument '-1' found with the tip itself removed.
    --flag=-1 was the only spelling that worked, and it is not the spelling the help text or the docs suggest.
    Twenty of the affected arguments declare default_value_t = -1, so their own default was a value the parser refused when an operator typed it by hand.
    allow_negative_numbers = true is now set on all 24 of them, covering set-retention, set-backlog-quota, set-dispatch-rate, set-subscription-dispatch-rate, set-replicator-dispatch-rate and set-publish-rate under both admin namespaces and admin topics.
    The opt-in gates on the shape of the token rather than on the leading hyphen alone — unlike the broader allow_hyphen_values — so a malformed --time-minutes -1abc is still rejected and the = form keeps working unchanged.