Skip to content

Commit 370e187

Browse files
committed
fix(qwp): split role-reject observability out of onDurableAckUnavailable (M10)
Unreleased commits d0a794a/4200697 made onDurableAckUnavailable fire for BOTH the durable-ack capability gap and the transient all-replica window, passing whichever mode-local counter was current with no discriminator. A listener written against the released 1.3.4 contract (capability-gap-only, counter marching toward the 16-attempt escalation) saw a non-monotonic, ambiguous stream: alerting keyed on "attemptNumber approaching 16" false-positived on role-reject windows (which never escalate, Invariant B) and could miss real budget pressure. Split the streams before 1.3.5 ships the break: - new default method onPrimaryUnavailable(slotPath, attemptNumber) carries the role-reject count (retried indefinitely, never escalates); the no-op default keeps 1.3.4 implementors source/binary compatible - onDurableAckUnavailable reverts to the released capability-gap-only semantics; its episode counter still restarts on an intervening role reject (topology churn grants a fresh settle budget), but the reset is now attributable to the onPrimaryUnavailable delivery between episodes - budget/episode/escalation logic untouched (Invariant B tests unmodified except for the per-stream assertion split)
1 parent 2986827 commit 370e187

3 files changed

Lines changed: 112 additions & 22 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,9 @@ public WebSocketClient connectWithDurableAckRetry() {
274274
BackgroundDrainerListener l = listener;
275275
if (l != null) {
276276
try {
277-
l.onDurableAckUnavailable(slotPath, roleRejectAttempts);
277+
l.onPrimaryUnavailable(slotPath, roleRejectAttempts);
278278
} catch (Throwable cb) {
279-
LOG.warn("drainer listener onDurableAckUnavailable threw: {}",
279+
LOG.warn("drainer listener onPrimaryUnavailable threw: {}",
280280
cb.getMessage());
281281
}
282282
}

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainerListener.java

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,42 @@ public interface BackgroundDrainerListener {
5757
void onDurableAckPersistentFailure(String slotPath, int totalAttempts, long elapsedMillis);
5858

5959
/**
60-
* Fired when a connect sweep found durable ack unavailable — either a
61-
* genuine capability gap ({@code QwpDurableAckMismatchException}: an
62-
* endpoint upgrades but does not advertise durable ack) or a transient
63-
* all-replica failover window (role reject). The drainer will back off
64-
* and retry; this callback is purely observability. Source data stays
65-
* pinned regardless because the loop runs in {@code durableAckMode=true}
66-
* and only trims on STATUS_DURABLE_ACK.
60+
* Fired when a connect sweep hit a genuine durable-ack capability gap
61+
* ({@code QwpDurableAckMismatchException}: an endpoint upgrades but does
62+
* not advertise durable ack). The drainer will back off and retry within
63+
* its settle budget; this callback is purely observability. Source data
64+
* stays pinned regardless because the loop runs in
65+
* {@code durableAckMode=true} and only trims on STATUS_DURABLE_ACK.
66+
* A transient all-replica failover window (role reject) never fires this
67+
* callback — it is surfaced through {@link #onPrimaryUnavailable}.
6768
*
6869
* @param slotPath slot the drainer is processing
69-
* @param attemptNumber 1-based attempt count within the current mode:
70-
* the running role-reject count for a transient
71-
* window, or the attempt number within the current
72-
* capability-gap episode (restarts when a role
73-
* reject resets the episode)
70+
* @param attemptNumber 1-based attempt number within the current
71+
* capability-gap EPISODE. The counter restarts when
72+
* an intervening role reject resets the episode —
73+
* topology churn grants the next gap a fresh settle
74+
* budget, which is correct behavior — and with the
75+
* streams separated the reset's cause is visible as
76+
* an {@link #onPrimaryUnavailable} delivery between
77+
* the two episodes
7478
*/
7579
void onDurableAckUnavailable(String slotPath, int attemptNumber);
80+
81+
/**
82+
* Fired when a connect sweep found every reachable endpoint to be a
83+
* REPLICA — a transient all-replica failover window (role reject). A
84+
* replica is promotable and a primary will reappear, so the drainer
85+
* retries indefinitely under Invariant B: this condition NEVER escalates
86+
* and is never followed by {@link #onDurableAckPersistentFailure}. Runs
87+
* on the drainer thread; implementations must not block. The no-op
88+
* default keeps every implementor of the released 1.3.4 contract source-
89+
* and binary-compatible.
90+
*
91+
* @param slotPath slot the drainer is processing
92+
* @param attemptNumber 1-based running role-reject count within the
93+
* current connect loop (resets across connect
94+
* re-entries)
95+
*/
96+
default void onPrimaryUnavailable(String slotPath, int attemptNumber) {
97+
}
7698
}

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/BackgroundDrainerDurableAckRetryTest.java

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444

4545
import java.nio.file.Paths;
4646
import java.util.ArrayList;
47+
import java.util.Arrays;
4748
import java.util.Collections;
4849
import java.util.List;
4950
import java.util.concurrent.CountDownLatch;
@@ -562,14 +563,19 @@ public void testRoleRejectChurnDoesNotConsumeCapabilityGapBudgetInvariantB() thr
562563
cap, listener.lastPersistentTotalAttempts.get());
563564
assertEquals("full settle budget must be granted after the transient window",
564565
roleRejects + cap, factory.attempts());
565-
// Per-mode attempt numbering: 1..20 for the transient window, then a
566-
// fresh 1..15 for the gap episode (the 16th fires persistent-failure).
567-
assertEquals(roleRejects + cap - 1, listener.unavailableAttempts.size());
566+
// M10 split: the transient all-replica window lands on the
567+
// onPrimaryUnavailable stream (1..20), the capability-gap episode
568+
// on onDurableAckUnavailable (1..15; the 16th fires
569+
// persistent-failure instead). Neither stream sees the other's
570+
// counter, so a listener alerting on "attemptNumber approaching
571+
// the cap" no longer false-positives on role-reject churn.
572+
assertEquals(roleRejects, listener.primaryUnavailableAttempts.size());
568573
for (int i = 0; i < roleRejects; i++) {
569-
assertEquals(Integer.valueOf(i + 1), listener.unavailableAttempts.get(i));
574+
assertEquals(Integer.valueOf(i + 1), listener.primaryUnavailableAttempts.get(i));
570575
}
576+
assertEquals(cap - 1, listener.unavailableAttempts.size());
571577
for (int i = 0; i < cap - 1; i++) {
572-
assertEquals(Integer.valueOf(i + 1), listener.unavailableAttempts.get(roleRejects + i));
578+
assertEquals(Integer.valueOf(i + 1), listener.unavailableAttempts.get(i));
573579
}
574580
assertTrue(Files.exists(slotPath + "/" + OrphanScanner.FAILED_SENTINEL_NAME));
575581
});
@@ -646,10 +652,62 @@ public void testRoleRejectResetsCapabilityGapEpisode() throws Exception {
646652
cap, listener.lastPersistentTotalAttempts.get());
647653
// 15 gap + 1 role reject + 16 gap = 32 sweeps total.
648654
assertEquals(2 * cap, factory.attempts());
655+
// M10 split, per-stream: the DA stream carries both episodes'
656+
// per-episode numbering (1..15, then 1..15 again -- the second
657+
// episode's 16th attempt fires persistent-failure instead), and
658+
// the reset between them is attributable: exactly one role reject
659+
// on the primary stream. Before the split the reset was an
660+
// ambiguous non-monotonic drop in a single stream.
661+
List<Integer> expectedDaStream = new ArrayList<>();
662+
for (int episode = 0; episode < 2; episode++) {
663+
for (int i = 1; i <= cap - 1; i++) {
664+
expectedDaStream.add(i);
665+
}
666+
}
667+
assertEquals(expectedDaStream, listener.unavailableAttempts);
668+
assertEquals(Collections.singletonList(1), listener.primaryUnavailableAttempts);
649669
assertTrue(Files.exists(slotPath + "/" + OrphanScanner.FAILED_SENTINEL_NAME));
650670
});
651671
}
652672

673+
@Test
674+
public void testRoleRejectAndCapabilityGapLandOnSeparateStreams() throws Exception {
675+
assertMemoryLeak(() -> {
676+
// M10 discriminator: gap -> role reject -> gap -> success. The
677+
// released 1.3.4 contract fed BOTH conditions to
678+
// onDurableAckUnavailable, so this script produced the ambiguous
679+
// stream [1, 1, 1] -- a listener could not tell a budget-bound
680+
// capability-gap episode from a never-escalating role-reject
681+
// window, and could not see WHY the episode counter reset. With
682+
// the split, the DA stream carries only the two one-attempt gap
683+
// episodes ([1, 1] -- the reset stays visible) and the role
684+
// reject that caused the reset lands on the primary stream ([1]).
685+
CountingListener listener = new CountingListener();
686+
AtomicInteger sweeps = new AtomicInteger();
687+
ScriptedFactory factory = ScriptedFactory.failingTimes(3, () -> {
688+
if (sweeps.incrementAndGet() == 2) {
689+
return new QwpIngressRoleRejectedException(
690+
QwpIngressRoleRejectedException.ROLE_REPLICA, "127.0.0.1", 9000);
691+
}
692+
return new QwpDurableAckMismatchException("h", 1234, "primary");
693+
});
694+
BackgroundDrainer drainer = newDrainer(factory);
695+
drainer.setListener(listener);
696+
WebSocketClient out = drainer.connectWithDurableAckRetry();
697+
assertSame(factory.successSentinel(), out);
698+
assertEquals(4, factory.attempts());
699+
assertEquals("DA stream must carry only the gap episodes, each"
700+
+ " restarting at 1 after the role-reject reset",
701+
Arrays.asList(1, 1), listener.unavailableAttempts);
702+
assertEquals("role reject must land on the primary stream",
703+
Collections.singletonList(1), listener.primaryUnavailableAttempts);
704+
assertEquals(Collections.singletonList(slotPath), listener.primaryUnavailableSlotPaths);
705+
assertEquals(BackgroundDrainer.DrainOutcome.PENDING, drainer.outcome());
706+
assertEquals(0, listener.persistentFailures.get());
707+
assertFalse(Files.exists(slotPath + "/" + OrphanScanner.FAILED_SENTINEL_NAME));
708+
});
709+
}
710+
653711
@Test
654712
public void testTransportErrorDoesNotResetCapabilityGapEpisode() throws Exception {
655713
assertMemoryLeak(() -> {
@@ -788,9 +846,11 @@ public void testRoleRejectGrantsFreshWallClockToNextGapEpisode() {
788846
0, listener.persistentFailures.get());
789847
assertFalse("no .failed sentinel: both gap episodes stayed inside their budgets",
790848
Files.exists(slotPath + "/" + OrphanScanner.FAILED_SENTINEL_NAME));
791-
// Per-mode attempt numbering across the reset: gaps 1,2 -- role reject
792-
// 1 -- fresh gap episode 1,2.
793-
assertEquals(java.util.Arrays.asList(1, 2, 1, 1, 2), listener.unavailableAttempts);
849+
// Per-stream attempt numbering across the reset (M10 split): the DA
850+
// stream carries gaps 1,2 then the fresh episode's 1,2; the role
851+
// reject that restarted the episode lands on the primary stream.
852+
assertEquals(Arrays.asList(1, 2, 1, 2), listener.unavailableAttempts);
853+
assertEquals(Collections.singletonList(1), listener.primaryUnavailableAttempts);
794854
}
795855

796856
@Test
@@ -907,6 +967,8 @@ private static final class CountingListener implements BackgroundDrainerListener
907967
final AtomicInteger lastPersistentElapsedMs = new AtomicInteger(-1);
908968
final AtomicInteger lastPersistentTotalAttempts = new AtomicInteger(-1);
909969
final AtomicInteger persistentFailures = new AtomicInteger();
970+
final List<Integer> primaryUnavailableAttempts = new ArrayList<>();
971+
final List<String> primaryUnavailableSlotPaths = new ArrayList<>();
910972
final List<Integer> unavailableAttempts = new ArrayList<>();
911973
final List<String> unavailableSlotPaths = new ArrayList<>();
912974

@@ -922,6 +984,12 @@ public synchronized void onDurableAckUnavailable(String slotPath, int attemptNum
922984
unavailableSlotPaths.add(slotPath);
923985
unavailableAttempts.add(attemptNumber);
924986
}
987+
988+
@Override
989+
public synchronized void onPrimaryUnavailable(String slotPath, int attemptNumber) {
990+
primaryUnavailableSlotPaths.add(slotPath);
991+
primaryUnavailableAttempts.add(attemptNumber);
992+
}
925993
}
926994

927995
/**

0 commit comments

Comments
 (0)