Skip to content

Commit 0246526

Browse files
sonus21claude
andcommitted
fix(nats): correct delayed-queue scheduling implementation (ADR-51)
The enqueueWithDelay path was completely broken: it used a made-up header (Nats-Next-Deliver-Time) that NATS ignores, published to the work subject instead of a scheduler subject, and the pull consumer had no filter so it read scheduler entries before the scheduled time. Correct implementation (NATS >= 2.12, ADR-51): - Replace HDR_NEXT_DELIVER_TIME with Nats-Schedule, Nats-Schedule-Target, and Nats-Rollup: sub — the three headers the server actually requires - Publish to a per-message scheduler subject (<work>.sched.<msgId>); NATS fires the triggered message to the work subject at the due time - Extend stream subjects to include the <work>.sched.* wildcard pattern - Set filterSubject on pull consumers so they only receive work-subject messages and never see scheduler entries early Infrastructure fixes that were hiding the bug: - AbstractJetStreamIT: replace @testcontainers(disabledWithoutDocker=true) with assumeTrue(DockerClientFactory.isDockerAvailable()) inside @BeforeAll so NATS_RUNNING=1 works even without Docker (annotation was vetoing the class before @BeforeAll ran, silently skipping all ITs) - CI: bump nats-server from v2.10.22 → v2.12.0 so scheduling assumeTrue guards stop silently skipping and actually gate the build - NatsProvisioner: add filterSubject overload to ensureConsumer; add allowMessageSchedules flag to stream creation / upgrade path - AllowMessageSchedulesEnforcementIT: new IT proving server enforces the allow_msg_schedules flag with Nats-Schedule (error 10189 without it) All 306 rqueue-nats tests pass including 3 new scheduling ITs that verify messages are held until the scheduled time then delivered. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent ace5e3c commit 0246526

11 files changed

Lines changed: 465 additions & 82 deletions

File tree

.github/workflows/java-ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ jobs:
365365
# instead of pulling a Testcontainers image.
366366
- name: Install nats-server
367367
run: |
368-
NATS_VERSION=v2.10.22
368+
NATS_VERSION=v2.12.0
369369
curl -sSL "https://github.com/nats-io/nats-server/releases/download/${NATS_VERSION}/nats-server-${NATS_VERSION}-linux-amd64.tar.gz" \
370370
| tar -xz -C /tmp
371371
sudo mv "/tmp/nats-server-${NATS_VERSION}-linux-amd64/nats-server" /usr/local/bin/nats-server

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ Never remove or increment the base version number. The human decides when the of
3131
<claude-mem-context>
3232
# Memory Context
3333

34-
# [rqueue] recent context, 2026-05-09 2:28pm GMT+5:30
34+
# [rqueue] recent context, 2026-05-09 9:08pm GMT+5:30
3535

3636
No previous sessions found.
3737
</claude-mem-context>

rqueue-nats/src/main/java/com/github/sonus21/rqueue/nats/internal/NatsProvisioner.java

Lines changed: 90 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.io.IOException;
3131
import java.time.Duration;
3232
import java.util.List;
33-
import java.util.Set;
3433
import java.util.concurrent.ConcurrentHashMap;
3534
import java.util.logging.Level;
3635
import java.util.logging.Logger;
@@ -53,8 +52,8 @@ public class NatsProvisioner {
5352
private final ConcurrentHashMap<String, KeyValue> kvCache = new ConcurrentHashMap<>();
5453
private final ConcurrentHashMap<String, Object> kvLocks = new ConcurrentHashMap<>();
5554

56-
// stream name → provisioned (set membership acts as the boolean flag)
57-
private final Set<String> streamsDone = ConcurrentHashMap.newKeySet();
55+
// stream name → schedulingEnabled (true if the stream was created/updated with allowMessageSchedules)
56+
private final ConcurrentHashMap<String, Boolean> streamsDone = new ConcurrentHashMap<>();
5857
private final ConcurrentHashMap<String, Object> streamLocks = new ConcurrentHashMap<>();
5958

6059
// "streamName/requestedConsumerName" → actual consumer name (may differ for stale-rebind)
@@ -63,7 +62,7 @@ public class NatsProvisioner {
6362

6463
/**
6564
* Minimum NATS server version that supports server-side message scheduling via the
66-
* {@code Nats-Next-Deliver-Time} JetStream publish header (ADR-51).
65+
* {@code Nats-Schedule} JetStream publish header (ADR-51).
6766
*/
6867
public static final String SCHEDULING_MIN_VERSION = "2.12.0";
6968

@@ -146,12 +145,18 @@ public KeyValue ensureKv(String bucketName, Duration ttl)
146145
* use {@link #ensureStream(String, List, QueueType)} instead.
147146
*/
148147
public void ensureStream(String streamName, List<String> subjects) {
149-
ensureStream(streamName, subjects, QueueType.QUEUE, null);
148+
ensureStream(streamName, subjects, QueueType.QUEUE, null, false);
150149
}
151150

152-
/** See {@link #ensureStream(String, List, QueueType, String)}. */
151+
/** See {@link #ensureStream(String, List, QueueType, String, boolean)}. */
153152
public void ensureStream(String streamName, List<String> subjects, QueueType queueType) {
154-
ensureStream(streamName, subjects, queueType, null);
153+
ensureStream(streamName, subjects, queueType, null, false);
154+
}
155+
156+
/** See {@link #ensureStream(String, List, QueueType, String, boolean)}. */
157+
public void ensureStream(
158+
String streamName, List<String> subjects, QueueType queueType, String description) {
159+
ensureStream(streamName, subjects, queueType, description, false);
155160
}
156161

157162
/**
@@ -168,20 +173,35 @@ public void ensureStream(String streamName, List<String> subjects, QueueType que
168173
* {@code nats stream info}). Callers should pass the rqueue queue name so operators can map a
169174
* stream back to the queue that created it; pass {@code null} to skip.
170175
*
176+
* <p>{@code allowSchedules} must be {@code true} when the stream will receive messages published
177+
* with the {@code Nats-Schedule} header (ADR-51). Only callers that perform delayed
178+
* enqueue should pass {@code true}; regular enqueue callers should pass {@code false} (or use the
179+
* shorter overloads). Equivalent to the CLI flag {@code --allow-schedules}.
180+
*
171181
* <p>Hits the NATS backend at most once per stream name per process lifetime; subsequent calls
172-
* return immediately from the in-process cache. If the stream already exists with a different
182+
* return immediately from the in-process cache. If {@code allowSchedules=true} is later requested
183+
* for a stream that was previously created without that flag, the stream is updated in place via
184+
* {@link JetStreamManagement#updateStream}. If the stream already exists with a different
173185
* retention policy, a WARNING is logged and the existing config is left untouched.
174186
*/
175187
public void ensureStream(
176-
String streamName, List<String> subjects, QueueType queueType, String description) {
177-
if (streamsDone.contains(streamName)) {
188+
String streamName,
189+
List<String> subjects,
190+
QueueType queueType,
191+
String description,
192+
boolean allowSchedules) {
193+
// Fast-path: already provisioned with at least as many capabilities as requested.
194+
Boolean cached = streamsDone.get(streamName);
195+
if (cached != null && (cached || !allowSchedules)) {
178196
return;
179197
}
180198
Object lock = streamLocks.computeIfAbsent(streamName, k -> new Object());
181199
synchronized (lock) {
182-
if (streamsDone.contains(streamName)) {
200+
cached = streamsDone.get(streamName);
201+
if (cached != null && (cached || !allowSchedules)) {
183202
return;
184203
}
204+
boolean enableSchedules = allowSchedules && schedulingSupported;
185205
try {
186206
StreamInfo existing = safeGetStreamInfo(streamName);
187207
RetentionPolicy desired =
@@ -199,6 +219,11 @@ public void ensureStream(
199219
.storageType(sd.getStorage())
200220
.retentionPolicy(desired)
201221
.compressionOption(CompressionOption.S2);
222+
if (enableSchedules) {
223+
// Enable server-side message scheduling (ADR-51 / Nats-Schedule header).
224+
// Equivalent to: nats stream add MY_STREAM --allow-schedules
225+
b.allowMessageSchedules(true);
226+
}
202227
if (description != null && !description.isEmpty()) {
203228
b.description(description);
204229
}
@@ -223,30 +248,63 @@ public void ensureStream(
223248
+ " — leaving existing config in place.",
224249
new Object[] {streamName, actual, desired});
225250
}
251+
// If scheduling was previously not enabled but is now required, update the stream.
252+
if (enableSchedules && !existing.getConfiguration().getAllowMsgSchedules()) {
253+
StreamConfiguration updated = StreamConfiguration.builder(existing.getConfiguration())
254+
.allowMessageSchedules(true)
255+
.build();
256+
jsm.updateStream(updated);
257+
log.log(Level.INFO,
258+
"Stream ''{0}'' updated to enable message scheduling (ADR-51).", streamName);
259+
}
226260
}
227261
} catch (IOException | JetStreamApiException e) {
228262
throw new RqueueNatsException(
229263
"Failed to ensure stream '" + streamName + "' for subjects " + subjects, e);
230264
}
231-
streamsDone.add(streamName);
265+
streamsDone.put(streamName, enableSchedules);
232266
}
233267
}
234268

235269
/**
236270
* Ensure a durable pull consumer exists, returning the consumer name.
237271
* Hits the NATS backend at most once per (stream, consumer) pair per process lifetime.
238272
*
239-
* <p>No filter subject is set on the consumer: each queue already has its own dedicated stream
240-
* with a single subject, so a filter would be redundant. More importantly, omitting the filter
241-
* allows multiple independent consumer groups (fan-out) to coexist on the same stream — NATS
242-
* rejects two consumers with the same filter subject (error 10100) regardless of retention type.
273+
* <p>Overload without a filter subject: used when the stream has only the work subject so the
274+
* filter would be redundant, or when multiple independent consumer groups (fan-out) must coexist
275+
* on a Limits-retention stream (NATS rejects two consumers with the same filter subject, error
276+
* 10100). For WorkQueue streams that also carry scheduler subjects ({@code .sched.*}) a filter
277+
* subject MUST be supplied via
278+
* {@link #ensureConsumer(String, String, String, Duration, long, long)} so the consumer only
279+
* receives work-subject messages and does not accidentally pick up scheduler entries.
243280
*/
244281
public String ensureConsumer(
245282
String streamName,
246283
String consumerName,
247284
Duration ackWait,
248285
long maxDeliver,
249286
long maxAckPending) {
287+
return ensureConsumer(streamName, consumerName, null, ackWait, maxDeliver, maxAckPending);
288+
}
289+
290+
/**
291+
* Ensure a durable pull consumer exists with an optional subject filter, returning the consumer
292+
* name. Hits the NATS backend at most once per (stream, consumer) pair per process lifetime.
293+
*
294+
* <p>{@code filterSubject} — when non-null, sets the consumer's filter subject so it only
295+
* receives messages published to that subject. Required when the stream carries both work subjects
296+
* and scheduler subjects ({@code .sched.*}): without a filter the consumer reads scheduler
297+
* entries before the scheduled time, delivering the message early. Pass {@code null} for streams
298+
* that do not use NATS scheduling, or where fan-out across multiple consumers is needed (Limits
299+
* retention).
300+
*/
301+
public String ensureConsumer(
302+
String streamName,
303+
String consumerName,
304+
String filterSubject,
305+
Duration ackWait,
306+
long maxDeliver,
307+
long maxAckPending) {
250308
String cacheKey = streamName + "/" + consumerName;
251309
String cached = consumerCache.get(cacheKey);
252310
if (cached != null) {
@@ -259,7 +317,7 @@ public String ensureConsumer(
259317
return cached;
260318
}
261319
String actual =
262-
doEnsureConsumer(streamName, consumerName, ackWait, maxDeliver, maxAckPending);
320+
doEnsureConsumer(streamName, consumerName, filterSubject, ackWait, maxDeliver, maxAckPending);
263321
consumerCache.put(cacheKey, actual);
264322
return actual;
265323
}
@@ -268,6 +326,7 @@ public String ensureConsumer(
268326
private String doEnsureConsumer(
269327
String streamName,
270328
String consumerName,
329+
String filterSubject,
271330
Duration ackWait,
272331
long maxDeliver,
273332
long maxAckPending) {
@@ -295,16 +354,20 @@ private String doEnsureConsumer(
295354
throw new RqueueNatsException("Consumer '" + consumerName + "' on stream '" + streamName
296355
+ "' does not exist and autoCreateConsumers=false");
297356
}
298-
jsm.addOrUpdateConsumer(
299-
streamName,
300-
ConsumerConfiguration.builder()
301-
.durable(consumerName)
302-
.ackPolicy(AckPolicy.Explicit)
303-
.deliverPolicy(DeliverPolicy.All)
304-
.ackWait(ackWait)
305-
.maxDeliver(maxDeliver)
306-
.maxAckPending(maxAckPending)
307-
.build());
357+
ConsumerConfiguration.Builder ccBuilder = ConsumerConfiguration.builder()
358+
.durable(consumerName)
359+
.ackPolicy(AckPolicy.Explicit)
360+
.deliverPolicy(DeliverPolicy.All)
361+
.ackWait(ackWait)
362+
.maxDeliver(maxDeliver)
363+
.maxAckPending(maxAckPending);
364+
if (filterSubject != null && !filterSubject.isEmpty()) {
365+
// Filter to the work subject only so that scheduler entries (published to
366+
// <workSubject>.sched.*) are not delivered to this consumer before the scheduled time.
367+
// The NATS scheduler fires the triggered message to workSubject when the time arrives.
368+
ccBuilder.filterSubject(filterSubject);
369+
}
370+
jsm.addOrUpdateConsumer(streamName, ccBuilder.build());
308371
return consumerName;
309372
} catch (JetStreamApiException e) {
310373
throw new RqueueNatsException(

0 commit comments

Comments
 (0)