Repository navigation
PrefixIdentifiers() doesn't prefix the dead-letter, response, retry and control objects that Wolverine creates for itself #3700
Replies: 3 comments
|
@bittercoder Hey man, why don't you strongly consider this: https://jasperfx.net. You're here quite a bit with a set of issues that I don't think many other people would even care about. Response/system topic names have some randomness to them, no issue there. |
I updated the issue description to make the concern/ intent (Which is low) a bit clearer - mostly just spotted this issue while working on something else, but it's not a blocker for us, just seems a little inconsistent in docs. vs implementation? In terms of JasperFX support subscription - we're pretty early stage in rolling out Wolverine at this workplace, so I think we need more adoption / workloads using it before floating that idea up, but suspect we'll definitely be reaching for it at some point in the next year 😁 |
|
@bittercoder That was partially a brush back pitch asking you to slow down a bit on reports here. The unfortunate trend for me is that people doing PoC work have weirder and more unusual questions or complaints than people just building systems do. I think I'm turning this into a discussion honestly to clear it out of the way. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary
PrefixIdentifiers()namespaces user-declared endpoints, but not the broker objects Wolverine creates for its own use (dead-letter queues/topics, response/reply endpoints, retry queues, control queues). So two apps sharing a broker with different prefixes get isolated application queues while still sharing their dead-letter and reply infrastructure.I originally hit this on GCP Pub/Sub, where the docs say the prefix applies to "every topic and subscription name created by Wolverine" (
docs/guide/messaging/transports/gcp-pubsub/index.md:77). Having looked at the other transports, I think none of them currently behave the way that's described, so we could either soften the docs or make the implementation match. My preference is the latter, but behind an opt-in flag since it changes provisioned resource names.Current behaviour
BrokerTransport.MaybeCorrectName(src/Wolverine/Transports/BrokerTransport.cs:42) only affects a name if the call site calls it, andBrokerTransportdoesn't route anything through it itself. Every call site I could find is a user-facing config method (ListenTo*,To*, routing conventions, partitioned topologies). NeithertryBuildSystemEndpoints(:147) nor theendpoints()DLQ derivation hook (:124) goes through it in any transport.The shape where this bites: two developers both running the same service against one shared GCP project, one on
PrefixIdentifiers("dev-john")and one onPrefixIdentifiers("dev-jane"), both withEnableDeadLettering(). Their application topics come out correctly asdev-john.orders/dev-jane.orders, but both attach a subscription to the samewlvrn.dead-lettertopic, and Pub/Sub fans out to every attached subscription, so each of them receives a copy of the other's dead letters.AssignedNodeNumberdoesn't disambiguate the response topic either, since each dev's local durability storage assigns 1.Names that skip the prefix
GCP Pub/Sub (
src/Transports/GCP/Wolverine.Pubsub/)wlvrn.dead-letter, the DLQ topic (PubsubTransport.cs:15, assignedPubsubEndpoint.cs:62, materialisedPubsubTransport.cs:107)wlvrn.responses.{node}, the response topic (PubsubTransport.cs:137,142)PubsubEndpoint.cs:50-56), so for both of the above the topic and subscription names disagree.RabbitMQ (
src/Transports/RabbitMQ/Wolverine.RabbitMQ/)wolverine-dead-letter-queue, used as the DLQ queue, exchange and binding key (defaultsDeadLetterQueue.cs:28,46,58, declaredRabbitMqTransport.cs:326-332)x-dead-letter-exchangeargument on prefixed application queues points at the unprefixed exchange (RabbitMqQueue.cs:315), so the isolation holds for normal traffic and not for failureswolverine.control.{node}(RabbitMqTransportExpression.cs:383)wolverine.response.{Guid}(RabbitMqTransport.cs:300), which is unique anyway because of the GuidAzure Service Bus (
src/Transports/Azure/Wolverine.AzureServiceBus/)wolverine.response.{ServiceName}.{node}(AzureServiceBusTransport.cs:202), the one ASB system name that gets neither prefix norSanitizeIdentifierwolverine.retries.{ServiceName}(:214)wolverine.control.{node}(AzureServiceBusConfiguration.cs:251), which has noServiceNamein it either, so it collides across unrelated appswolverine-dead-letter-queue(AzureServiceBusQueue.cs:201, derived:266-270).AzureServiceBusDeadLetterQueueListenerdrains this, so one host will pull another's dead letters into its own storage.Amazon SQS (
src/Transports/AWS/Wolverine.AmazonSqs/)wolverine-response-{servicename}-{node}(AmazonSqsTransport.cs:169-171)wolverine-control-{node}(AmazonSqsTransportConfiguration.cs:244-246)wolverine-dead-letter-queue(:16,31,AmazonSqsQueue.cs:128-132, derived:145-146). This is wired as an AWS redrive target, so AWS itself merges both apps' failures into it.CleanupOrphanedSystemQueuesAsyncscanswolverine-response-/wolverine-control-across the whole account (:205) and deletes matches whosewolverine:last-activetag is stale or missing (:222-233). I think that means two devs sharing an account can already delete each other's system queues, and it also means a fix would need to prefix the scan at the same time as the names, or cleanup quietly stops matching.Amazon SNS generates no system names. One adjacent gap:
SubscribeSqsQueue(name)uses the name verbatim (AmazonSnsSubscriberConfiguration.cs:80-81), where I think it wants the SQS transport'sMaybeCorrectNameviae.Parent.SQS.Transports where PrefixIdentifiers() doesn't do anything
Kafka, NATS and Redis expose
PrefixIdentifiers()throughBrokerExpressionbut never readIdentifierPrefix, so as far as I can tell it's a no-op for them, application names included:MaybeCorrectNamecall sites at all. Topics,wolverine.topics(KafkaTopic.cs:17), the DLQ topic (KafkaTransport.cs:62), retry-tier topics (KafkaRetryNaming.cs:51), consumer group ids (KafkaTransport.cs:103,203). Two apps landing on the same group id means Kafka splits the partitions between them.WithSubjectPrefix()(NatsTransportExpression.cs:33) just assigns the property. Subjects, pluswolverine.response.{node}(NatsTransport.cs:80). This one is documented as working atdocs/guide/messaging/transports/nats.md:458.wolverine.response.{service}.{node}(RedisTransport.cs:416),{StreamKey}:dead-letter,{StreamKey}:scheduled, and thewolverine-repliesconsumer group (:425).Pulsar and MQTT are
TransportBaserather thanBrokerTransport, so they don't have the API at all.Possible fix
Three parts, keen for a sanity check on the shape before I put a PR up:
Defaulting to false because prefixing these names changes provisioned resource names, so on by default would have existing apps create fresh objects on upgrade and leave whatever is in the old ones stranded. Backed by a
PrefixSystemIdentifiersproperty onBrokerTransportand aMaybeCorrectSystemName()helper that returns the identifier untouched unless opted in.For Kafka, NATS and Redis, a
virtual bool SupportsIdentifierPrefixingonBrokerTransportoverridden to false, withPrefixIdentifiers()throwingNotSupportedException. Feels better than leaving it silently doing nothing, though it does mean withdrawing something the NATS docs currently advertise, so happy to be talked out of that part.Leave explicitly-supplied names alone.
ConfigureDeadLettering("x"),DefaultDeadLetterQueueName("x"),DeclareQueue("x")etc. are treated as fully qualified today and I don't think that wants to change.Two things a fix needs to pick up along the way:
isSystemQueue()(RabbitMqQueue.cs:298) testsQueueName.StartsWith("wolverine."), which stops matching once system queues can carry a prefix.Importance
This isn't an issue for us right now in any production workloads, so mostly just flagging as a bit of a gotcha that might need to be addressed in the future (either doc changes to reflect implementation, or making the code more holistically support prefixes of system created topics/queues). Happy to put up a PR if that's useful, though feels like probably would want some guidance on what would be an apropriate approach too, particularly around backwards compatibility etc.
All reactions