Skip to content

Commit 8c387df

Browse files
holytshirtclaude
andcommitted
docs(adr): correct ADR 0057 — fallback exists on every RMQ delay surface
Addresses review feedback on PR #4106: - Constraints: corrected the claim that SendWithDelay had no scheduler fallback. Both Sync and Async producers (RmqMessageProducer) and the Async consumer (RmqMessageConsumer.RequeueAsync) already route through IAmAMessageScheduler when one is registered. Replaced the consumer snippet with the producer pattern (the more common surface) and cross-referenced all three call sites. - Decision: simplified Phase 1 — runtime work is already done; what remains is [Obsolete], precedence-rule documentation, and a migration guide. Added "(target version: TBD on acceptance)" to both phases. - Decision: replaced the broken docs/contents/RabbitMQConfiguration.md reference with a generic phrasing and a note that the location is to be confirmed during implementation (Brighter's RMQ user docs may live outside this repo). - Responsibilities: dropped the .agent_instructions internal link (harness-only directory, not public-facing). - Risks: reworded the precedence-ambiguity risk to point at the now-explicit rule in Decision/Phase 1 step 2 instead of duplicating the mitigation. - References: expanded the fallback-sites bullet to list all three call sites with line-number anchors; removed the .agent_instructions link. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 576cc43 commit 8c387df

1 file changed

Lines changed: 35 additions & 23 deletions

File tree

docs/adr/0057-rabbitmq-delayed-messaging-strategy.md

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,25 @@ The terminal working combination — RMQ 4.2 + plugin v4.2.0-rc.1 — is being a
2424

2525
### Constraints
2626

27-
- **Existing role available.** `IAmAMessageScheduler` is already a role in Brighter, with six production implementations: `Paramore.Brighter.MessageScheduler.{Aws, AWS.V4, Azure, Hangfire, Quartz, TickerQ}`, plus an `InMemoryScheduler`. Other transports without native broker-side delay (notably AWS classic where applicable) already use this role.
28-
- **Partial fallback already wired.** [`RmqMessageConsumer.RequeueAsync`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageConsumer.cs#L419) routes through the scheduler when `DelaySupported = false`:
27+
- **Existing role available.** `IAmAMessageScheduler` is already a role in Brighter, with six production implementations: `Paramore.Brighter.MessageScheduler.{Aws, AWS.V4, Azure, Hangfire, Quartz, TickerQ}`, plus an `InMemoryScheduler`. Other transports without native broker-side delay already use this role.
28+
- **Fallback already wired in both transports and on both surfaces.** Whenever a scheduler is registered and `DelaySupported = false`, both producers and the requeue path route through `IAmAMessageScheduler` today. Identical branching exists in [`RmqMessageProducer.SendWithDelayAsync`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageProducer.cs#L168) (Async), [`RmqMessageProducer.SendWithDelay`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessageProducer.cs#L155) (Sync), and [`RmqMessageConsumer.RequeueAsync`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageConsumer.cs#L419):
2929
```csharp
30-
if (DelaySupported || timeout <= TimeSpan.Zero)
31-
await rmqMessagePublisher.RequeueMessageAsync(message, _queueName, timeout.Value, cancellationToken);
30+
if (delay == TimeSpan.Zero || DelaySupported || Scheduler == null)
31+
{
32+
// plugin path: publish to the x-delayed-message exchange
33+
}
34+
else if (useSchedulerAsync)
35+
{
36+
var schedulerAsync = (IAmAMessageSchedulerAsync)Scheduler!;
37+
await schedulerAsync.ScheduleAsync(message, delay.Value, cancellationToken);
38+
}
3239
else
33-
await _requeueProducer!.SendWithDelayAsync(message, timeout, cancellationToken);
40+
{
41+
var schedulerSync = (IAmAMessageSchedulerSync)Scheduler!;
42+
schedulerSync.Schedule(message, delay.Value);
43+
}
3444
```
35-
The `SendWithDelay` producer surface, used directly by callers (not just by requeue), does **not** yet have an equivalent fallback.
45+
Runtime work for Option B is therefore essentially complete; what remains is signalling (deprecation), documentation, and eventual removal.
3646
- **OSS users must keep working.** Brighter cannot mandate Tanzu RabbitMQ — its OSS user base is significant and would be excluded by a commercial-only path.
3747
- **Migration window required.** Existing users with `Exchange.SupportDelay = true` should not break at the next minor release. A deprecation warning before removal is required.
3848
- **Wire format compatibility.** RabbitMQ's various delay implementations (community plugin, Tanzu plugin) all accept the `x-delay` header Brighter already emits. Decisions about *where* the delay is enforced are independent of *how* the message is shaped on the wire.
@@ -41,32 +51,32 @@ The terminal working combination — RMQ 4.2 + plugin v4.2.0-rc.1 — is being a
4151

4252
Combine **Option B** (route delay through `IAmAMessageScheduler`) with **Option E** (deprecate `Exchange.SupportDelay` and remove next major), with **Option D** (Tanzu) documented as a supported configuration for paying users.
4353

44-
### Phase 1 — Current major
54+
### Phase 1 — Current major (target version: TBD on acceptance)
4555

46-
1. Mark `Exchange.SupportDelay` `[Obsolete("Configure an IAmAMessageScheduler instead — the rabbitmq_delayed_message_exchange plugin is archived upstream and incompatible with RabbitMQ 4.3+. See ADR 0057.")]`.
47-
2. Extend the producer's `SendWithDelay` path to use `IAmAMessageScheduler` when one is registered, falling through to the plugin path only when no scheduler is registered and `SupportDelay = true` (preserves current behaviour for users who haven't migrated).
48-
3. Keep the plugin path operational against RMQ 4.2 + plugin v4.2.0-rc.1 (PR #4104 already adopts this pinning).
49-
4. Add a migration guide section to [`docs/contents/RabbitMQConfiguration.md`](../contents/RabbitMQConfiguration.md) showing how to register a scheduler and remove `SupportDelay`.
56+
1. **Mark `Exchange.SupportDelay` `[Obsolete]`** with text along the lines of: `"Configure an IAmAMessageScheduler instead — the rabbitmq_delayed_message_exchange plugin is archived upstream and incompatible with RabbitMQ 4.3+. See ADR 0057."`
57+
2. **Document the runtime precedence rule** explicitly. When both `Scheduler` is registered *and* `Exchange.SupportDelay = true`, the **scheduler wins** — this is how the existing producer code already behaves (see code in §Constraints), but the rule has not been documented externally and must be made explicit so users migrating in either direction know what to expect. Surface the rule in the `[Obsolete]` message text and in the migration guide.
58+
3. **Keep the plugin path operational** against RMQ 4.2 + plugin v4.2.0-rc.1 for users who haven't yet migrated. PR #4104 already adopts this pinning.
59+
4. **Write a migration guide** in the user-facing RabbitMQ configuration documentation (location to be confirmed during implementation — `docs/transports/` in this repo, or the public docs site, depending on where Brighter's RMQ guide currently lives). Include: how to register a scheduler, the precedence rule, and a comparison table of scheduler backends to help users pick (in-process vs distributed; polling interval; persistence).
5060

51-
### Phase 2 — Next major
61+
### Phase 2 — Next major (target version: TBD on acceptance)
5262

53-
1. Remove the plugin-aware code paths in `ExchangeConfigurationHelper`, the `x-delay` exchange declaration, and the `DelaySupported` branch in `RmqMessageConsumer.RequeueAsync`.
54-
2. Make `IAmAMessageScheduler` registration mandatory for any caller of `SendWithDelay` or `RequeueAsync(timeout: nonzero)` on RMQ. Throw a `ConfigurationException` at startup if delay is used without a registered scheduler.
55-
3. Stop bundling the delay plugin in the CI test image; pin to plain `rabbitmq:management`.
63+
1. **Remove the plugin-aware code paths** in `ExchangeConfigurationHelper`, the `x-delay` exchange declaration, and the `DelaySupported` branch in both producers and `RmqMessageConsumer.RequeueAsync`.
64+
2. **Make `IAmAMessageScheduler` registration mandatory** for any caller of `SendWithDelay` or `RequeueAsync(timeout: nonzero)` on RMQ. Throw a `ConfigurationException` at startup if delay is used without a registered scheduler.
65+
3. **Stop bundling the delay plugin** in the CI test image; pin to plain `rabbitmq:management`.
5666

5767
### Tanzu RabbitMQ support
5868

59-
For users on Tanzu, document — in [`docs/contents/RabbitMQConfiguration.md`](../contents/RabbitMQConfiguration.md)how to declare `x-queue-type: delayed` queues and rely on Tanzu's `rabbitmq_delayed_queue` plugin. No code change is required: Tanzu accepts the same `x-delay` header Brighter already emits, so Brighter's wire format is forward-compatible.
69+
For users on Tanzu, document how to declare `x-queue-type: delayed` queues and rely on Tanzu's `rabbitmq_delayed_queue` plugin. No code change is required: Tanzu accepts the same `x-delay` header Brighter already emits, so Brighter's wire format is forward-compatible. Documentation will live alongside the migration guide added in Phase 1, step 4.
6070

6171
### Responsibilities
6272

63-
Following Responsibility-Driven Design (per [.agent_instructions/design_principles.md](../../.agent_instructions/design_principles.md)):
73+
Following Responsibility-Driven Design:
6474

6575
- **Knowing the delay value** — the `Message` and its routing slip carry the requested delay. Unchanged.
66-
- **Deciding where to enforce delay** — currently split between `Exchange.SupportDelay` (broker enforces) and the absence of that flag (scheduler enforces, in the requeue path only). After Phase 2, this responsibility consolidates into a single *coordinator* role: the producer asks "is a scheduler registered?" and there is exactly one path.
76+
- **Deciding where to enforce delay** — currently split between `Exchange.SupportDelay` (broker enforces) and the absence of that flag (scheduler enforces). After Phase 2, this responsibility consolidates into a single *coordinator* role: the producer asks "is a scheduler registered?" and there is exactly one path.
6777
- **Doing the delay** — currently a *service-provider* role split between RabbitMQ-via-plugin (broker side) and `IAmAMessageScheduler` (Brighter side). After Phase 2, only the latter remains.
6878

69-
This consolidation matches the design-principles guidance: "There should be one — and preferably only one — obvious way to do it."
79+
This consolidation matches the long-standing principle in Brighter's design guidelines: "There should be one — and preferably only one — obvious way to do it."
7080

7181
## Consequences
7282

@@ -97,8 +107,8 @@ This consolidation matches the design-principles guidance: "There should be one
97107
**Risk**: Scheduler choice paralysis — users don't know which of six to pick.
98108
- **Mitigation**: Add a comparison table to the RMQ docs (in-process vs distributed, polling interval, persistence backend). Recommend TickerQ for in-process, Hangfire or Quartz for distributed deployments.
99109

100-
**Risk**: The Phase 1 fallback ordering (scheduler-if-registered, else plugin) introduces ambiguity for users who have *both* a scheduler and `SupportDelay = true`.
101-
- **Mitigation**: Document explicitly that registering a scheduler takes precedence; emit an `[Obsolete]` warning that says exactly this; surface it in `[Obsolete]` message text.
110+
**Risk**: The Phase 1 fallback ordering scheduler wins when both a scheduler and `SupportDelay = true` are configured — is current runtime behaviour but has never been documented. Users who configured both could be surprised when their scheduler unexpectedly runs.
111+
- **Mitigation**: Phase 1 step 2 makes this rule explicit in the migration guide and `[Obsolete]` message. The behaviour itself does not change — only the documentation.
102112

103113
## Alternatives Considered
104114

@@ -151,5 +161,7 @@ Tanzu RabbitMQ ships a closed-source replacement: a queue type `x-queue-type: de
151161
- TTL + DLX official docs: <https://www.rabbitmq.com/docs/ttl>, <https://www.rabbitmq.com/docs/dlx>
152162
- Tanzu delayed queues: <https://techdocs.broadcom.com/us/en/vmware-tanzu/data-solutions/tanzu-rabbitmq-ova/4-2/tanzu-rabbitmq-ova-virtual-machine/site-delayed-queues.html>
153163
- Brighter scheduler interface: [`src/Paramore.Brighter/IAmAMessageScheduler.cs`](../../src/Paramore.Brighter/IAmAMessageScheduler.cs)
154-
- Existing fallback site: [`src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageConsumer.cs`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageConsumer.cs) (the `else` branch in `RequeueAsync`)
155-
- Design principles applied: [.agent_instructions/design_principles.md](../../.agent_instructions/design_principles.md) (Responsibility-Driven Design — knowing/doing/deciding)
164+
- Existing fallback sites:
165+
- Async producer: [`src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageProducer.cs`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageProducer.cs#L168)
166+
- Sync producer: [`src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessageProducer.cs`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessageProducer.cs#L155)
167+
- Async consumer requeue: [`src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageConsumer.cs`](../../src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageConsumer.cs#L419)

0 commit comments

Comments
 (0)