Nservicebus: Endpoint instance ID -- how?

[SzymonPobiega]

Let's continue the discussion (started here) about endpoint instance IDs and how they can be generated. Just as a recap, the reasons for assigning an ID to endpoint instance might be following:

• Overcoming throughput limitation of individual queues
• Using callbacks
• Having logically significant endpoint instances (e.g. representing real-world objects)
• Other?

The question is, how to generate such an ID. Here's some options:

1. Don't generate. Make the user provide it.
2. Host provides it (e.g. azure host uses role instance ID, NSB host uses unique id stored in file on disk)
3. Plug-in (e.g. callbacks) provides it -- in this case a random ID generated each time endpoint starts
4. We use path like with HostID

Some of the forces that are at play here are:

1. In most cases users don't care
2. If we make users provide ID, they will hardcode it and cause bad behavior
3. Solutions that work well on-premise tend to not work for cloud
4. The ID should be as stable as possible to avoid creating many ghost queues
5. The ID should be as human-readable as possible so that operations can make sense of it

[SzymonPobiega]

@johnsimons @andreasohlund @danielmarbach @yvesgoeleven did I miss anything?

[johnsimons]

So after reviewing the PR I came to the conclusion that the only option we have is option 3, random id everytime the endpoint starts.
Here is my thinking, callbacks main usage is websites, majority of these are hosted in IIS, IIS has this wonderful thing called AppPools, by default AppPools recycle every x hours, everytime a AppPool recycles IIS starts a new instance of the endpoint and start shutting down the old one (for a small window there are 2+ instances running, competing consumers).
So hopefully as you can see from my little explanation if we do not random gen an id (in other words, random gen a callbacks queue) everytime the endpoint starts there is a chance of losing callbacks.

Disadvantage

• for transports that need extra elevation to create queues, it means the account that runs these endpoints that use callbacks need rights to create these "random" queues at runtime;
• for transports that do not support temp queues or auto-delete queues, there is an overhead of managing these extra queues;

Advantage

• no message loss;
• this one may sound funny but here it goes, because of the PITA of managing these extra queues + queue creation acls, it may actually push users to drop callbacks and design their systems correctly;
• works not just for scale out but also for side by side upgrades + IIS app pool recycles;

[andreasohlund]

I like the combination of 1 + 2 best

1 Don't generate. Make the user provide it.

This raise awareness (yes some won't care and hardcode) and is OPS friendly since IMO ops do not like guids in names in their infra. This also means that OPS can stipulate naming conventions.

2 Host provides it (e.g. azure host uses role instance ID, NSB host uses unique id stored in file on disk)

Layering no2 on top of no1 give the best of both worlds. A core that makes no assumptions since it doesn't have enough context. And hosts that applies sensible defaults based on its context.

NServiceBus.Host => machineName + windows service name

AzureHost => endpoint name + role instance id

NServiceBus.WindowsService.Bootstrap => machineName + endpoint name?

A furture NServiceBus.AspNetCore.Bootstrap => machineName + web site name (is there such a thing?)

[andreasohlund]

And it also have the benefit of making storage of the value a non issue.

Of course there can be an option to "just give me random queue names" if the user should want that. I think this might be attractive if the given transport does support temp queues and the user is running with permissions to create them. But lets face it, if you are on eg. the sqltransport this will not be an option.

[yvesgoeleven]

I'm with andreas:

Random instance id's are in general not a good idea,

• when instances are reprovisioned or scaled out, which happens often in azure, the id's would change, leading to rogue queues with potentially messages piling up in them.
• instance specific data is also stored (f.e. timeouts) and could be associated with these id's. Random id's would also lead to rogue data (aka loss of timeouts) in this case

Choosing an instance id is a hosting concern, we should make no assumptions about it in the core, let the host choose a default value that makes sense and allow the user to override it for special cases

[SzymonPobiega]

OK. So at first when I spiked the PR, I created 3 "levels" where instance ID can be provided: user, host and plug-in. The first two I think still make sense. The last one was there to support situations like Callbacks plug-in setting the ID generation strategy to random.

In the light of what @yvesgoeleven and @andreasohlund said, I think the plug-in level can be removed and the behavior should be:

• no instance ID by default
• host sets instance ID
• user can provide instance ID that overrides the host (as usual with most APIs where we provide some default)

Thoughts?

[andreasohlund]

:dancer:

[yvesgoeleven]

:+1:

[SzymonPobiega]

I've updated https://github.com/Particular/NServiceBus/pull/3374 according to my comment. Please take a look. When using from a host, it has to provide an instance of INeedInitialization which calls RegisterEndpointIdGenerator extension method. Should we create an explicit extension point for this purpose?

[andreasohlund]

What if the host call the method to set the generator before invoking user code. That way the user code will "overwrite" the host generator/value? (or is this to much of a hack)

[SzymonPobiega]

The idea was that user provided value has precedence over host provided, no
matter the order. Problem would be if the call was made to late. That func
would be registered after the id generation happens. But I can address that
too.

On Tuesday, 9 February 2016, Andreas Öhlund [email protected]
wrote:

What if the host call the method to set the generator before invoking user
code. That way the user code will "overwrite" the host generator/value? (or
is this to much of a hack)

—
Reply to this email directly or view it on GitHub
https://github.com/Particular/NServiceBus/issues/3406#issuecomment-181877904
.

[andreasohlund]

As an aside, if we let the hosts control the defaults why do we even need a
generator? why not just accept a string?

On Tue, Feb 9, 2016 at 3:13 PM, Szymon Pobiega [email protected]
wrote:

The idea was that user provided value has precedence over host provided, no
matter the order. Problem would be if the call was made to late. That func
would be registered after the id generation happens. But I can address that
too.

On Tuesday, 9 February 2016, Andreas Öhlund [email protected]
wrote:

What if the host call the method to set the generator before invoking
user
code. That way the user code will "overwrite" the host generator/value?
(or
is this to much of a hack)

—
Reply to this email directly or view it on GitHub
<
https://github.com/Particular/NServiceBus/issues/3406#issuecomment-181877904

.

—
Reply to this email directly or view it on GitHub
https://github.com/Particular/NServiceBus/issues/3406#issuecomment-181880458
.

[SzymonPobiega]

Hmmm... true. You might be right @andreasohlund :-)

[mauroservienti]

As a side note, from the onsite review I did at a customer:

• callback queue name (or instance id if that's the case) must be predictable if auto-generated cause the endpoint may not have permissions to create queues, Ops (or automatic deploys are responsible to create queues not the endpoint)
• callback queues should be created only if callbacks are enabled (right now the behavior is transport dependant and is causing a lot of issues to customers)

[SeanFeldman]

@mauroservienti could you please elaborate what kind of issues? Thank you.

[SzymonPobiega]

@mauroservienti the current approach in V6 is, whenever you specify the instance ID (a.k.a. discriminator), there will be an instance-specific queue created. If you don't specify, no queues are created/expected to exist.

[mauroservienti]

@SeanFeldman moving across environments.

If the endpoint has no permissions to create queues, the deploy process is responsible to create queues as the endpoint expects, so if the endpoint needs a callback queue the callback queue name must be predictable otherwise the automated deploy process cannot create the expected queues and the endpoint crashes at startup while trying to create the missing queue(s).
I'd prefer to avoid as much as we can to force the user to set the instance id, I can only imagine how easy is to mess that up.

[andreasohlund]

@Particular/platform-dev-squad this is another indication that we should consider Particular/PlatformDevelopment#621 since it will make this "just work". @mikeminutillo was there a call with @udidahan setup to make the decision?

[mikeminutillo]

@andreasohlund given that Udi is travelling and delivering ADSD next week I have sent him an email asking what we should do about it.

[yvesgoeleven]

What's the verdict on this one?

[andreasohlund]

Since https://github.com/Particular/PlatformDevelopment/issues/621 most likely won't go into v6 we need to add a temporary api to require endpoint instance id if callbacks/features needing it is enabled.

In short:

1. Features can do a "GetEndpointInstanceId" if they need
2. If not provided by the user that will throw an ex guiding the users to set it

Do we already have a api or should we add a configuration.EndpointInstanceId("") and then we provide doco on what to put there. Eg machine name + win service name for win services, rolename + roleinstance ids for azure etc?

[SzymonPobiega]

We currently have endpointConfig.ScaleOut().InstanceDiscriminator(string instanceId) and I really really don't like it. I prefer

config.EndpointInstanceId(string id);

or

config.EndpointInstanceId(Func<string> idGetter);

The latter suggest you should never ever hardcode that value but rather provide a function that retrieves it from some place (most likely configuration). We also need to support providing that ID from the host perspective.

@yvesgoeleven I know we agree that providing Azure role ID is not a responsibility of the transport but rather the host but is that doable in V6 time frame? Should the Azure/Windows host call config.EndpointIInstanceId() ?

[tmasternak]

I like config.EndpointInstanceId(Func<string> idGetter);. It expresses the intention better.

[SimonCropp]

Sorry but the function in no way implies that it shouldnt be hard coded. To me a function inplies that it will be called multiple times during endpoint execution and i am free to return a different value each time. That is how all our other func apis work

[andreasohlund]

Agree with @SimonCropp I think a string better conveys the intent since the value is "static" once configuration is done. A func seems to imply that it can be changed.

[danielmarbach]

What @SimonCropp said

[SzymonPobiega]

OK, I feel convinced by @SimonCropp .

[SzymonPobiega]

How about we combine it with #3495 for the V6 release and use following API

var config = new EndpointConfiguration("MyEndpoint");
noop -> Competing, non uniquely addressable
config.UniquelyAddressableInstance("MyInstance"); //Competing, unique instance ID, instance-specific queue
config.IndependentInstance("MyInstance"); //Indepent instances, no shared queue

cc @mauroservienti @yvesgoeleven @andreasohlund

[danielmarbach]

Like that

On Wednesday, March 16, 2016, Szymon Pobiega [email protected]
wrote:

How about we combine it with #3495
https://github.com/Particular/NServiceBus/issues/3495 for the V6
release and use following API

var config = new EndpointConfiguration("MyEndpoint");
noop -> Competing, non uniquely addressable
config.UniquelyAddressableInstance("MyInstance"); //Competing, unique instance ID, instance-specific queue
config.IndependentInstance("MyInstance"); //Indepent instances, no shared queue

cc @mauroservienti https://github.com/mauroservienti @yvesgoeleven
https://github.com/yvesgoeleven @andreasohlund
https://github.com/andreasohlund

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/Particular/NServiceBus/issues/3406#issuecomment-197297404

[andreasohlund]

I like it as well. I propose we go ahead with this and remove https://github.com/Particular/PlatformDevelopment/issues/621 from v6

[mikeminutillo]

I still think that Independent Instance should require Uniquely Addressable, not an Id. These are orthogonal concerns. Uniquely Addressable adds a new address (queue). Independent Instance requires the unique address, and then turns off the shared address.

[bording]

The way I see it, the three modes are:

• Shared queue
• Shared queue + per-instance queue
• per-instance queue

I'd like to see the naming of the modes make it obvious from just the name that the middle mode has both queues.

Or, since there are two queues involved, maybe that's the way the API should be organized?

var config = new EndpointConfiguration("MyEndpoint");
// do nothing and get just a shared queue
config.EnablePerInstanceQueue("MyInstanceID"); // adds a per-instance queue
config.DisableSharedQueue() // turns off the shared queue if you just want an per-instance queue

And then we can throw for invalid combos, such as disabling the shared queue and not enabling the per-instance queue. And subscriptions would always be for the shared queue unless it's been disabled.

I think I like this approach better since it makes it very obvious what each config option is doing, vs. needing to map some sort of named mode to a set of functionality

[mikeminutillo]

@bording I disagree for the reason I am against the v5 UniqueQueuePerEndpoingInstance() and UseSingleBrokerQueue(). For me this puts emphasis on which queues we are binding the endpoint to. That is our concern, not our customers. The fact that there are queues at all is an implementation detail. What is the business problem that the user is trying to solve by using this API?

[SzymonPobiega]

:+1: for not talking about queues. So @mikeminutillo are you suggesting something like:

var config = new EndpointConfiguration("MyEndpoint");
//so far only competing consumers
config.UniquelyAddressable("MyInstance");
//now we add the per-instance receiver for MyEndpoint-MyInstance queue
config.IndependentInstance();
//now we switch subscriptions to use the per-instance queue rather than shared queue and disable using of the shared queue

[mikeminutillo]

@SzymonPobiega :+1: I'm still not sure what the use case for Independent Instance is but I am happier with that design. Is it still possible to send a command to the logical MyEndpoint Endpoint? If so then could this be:

var config = new EndpointConfiguration("MyEndpoint");
config.UniquelyAddressable("MyInstance");
config.RouteAllEventsToEveryInstance();

Is that closer to a requirements level description of what is going on?

[bording]

I'm all for hiding implementation details when it makes sense, but I'm not sure I consider the name and number of queues created to be an implementation detail our users don't have to be concerned with.

Considering that they have to set up, maintain, and monitor their chosen transport infrastructure, this seems like an odd thing to be obscuring to me.

[SzymonPobiega]

Let's pause here until https://github.com/Particular/NServiceBus/issues/3495 is decided.

[andreasohlund]

Removing the v6 label, if https://github.com/Particular/NServiceBus/issues/3638 show that we need to do this please put it back

[SzymonPobiega]

@andreasohlund I believe the smallest thing we should do for V6 is change the

config.ScaleOut().InstanceDiscriminator(string);

to

config.InstanceId(string);

[mauroservienti]

Yeah, scale out has no meaning any more in this context

[andreasohlund]

I like it, raise a new issue for this?

[SzymonPobiega]

Raised https://github.com/Particular/NServiceBus/issues/3652. Closing.