SQL v3: Multi-instance support

[mauroservienti]

Task force: @tmasternak @weralabaj @justabitofcode @mauroservienti @SzymonPobiega

Summary

We decided to drop the multi-instance support for the V3 of the SQL Transport.
Given that this is a feature that many of our customers could be using, we want to provide our existing users with a proper migration path.
We actually have a working spike that enables the multi-instance support as it was in V2, we decided to ship it as a sample in our documentation.

Along with this decision we also strongly think that we should release a set of guidance on why multi-instance is a bad design decision and how to implement shovel-like solutions in a SQL transport based topology.

Plan of Attack

• Provide legacy multi-instance mode custom connection factory
• Create sample showing how to set it up => @justabitofcode
  • Add v5 version SQL Server multi-instance sample v2 docs.particular.net#1359
• Update doco on multi-database => @weralabaj Update docs for SQL Server multi-X docs.particular.net#1346
  • Document Queue Schema= in Connection String for Sql Transport Document Queue Schema= in Connection String for Sql Transport docs.particular.net#1304
  • Remove all "multi-database" expressions, replace with either "multi-catalog" or "multi-instance"
  • Guidance on when to use each, list advantages and disadvantages (remember about Sql Server transport upgrade guide from v5 to v6 docs.particular.net#1206 (comment))
  • Explain that we require DTC enabled now (https://github.com/Particular/NServiceBus.SqlServer/pull/161/files#r53755702)
• Upgrade guide for multi-instance @weralabaj
• Update exception message ttps://github.com/Particular/NServiceBus.SqlServer/blob/develop/src/NServiceBus.SqlServer/Legacy/MultiInstance/LegacySqlServerTransportInfrastructure.cs#L51 After recent changes in doco we don't need to change it, it's consistent with vocabulary used in doco (comment by @weralabaj)
• Update sample with Multi-instance + Outbox in v5 and link to explanation why it's not longer supported add v6 store-and-forward docs.particular.net#1168
• Documentation/Guidance around security ?? Moved to Guidance content on security in SQL Server transport docs.particular.net#1347.
• Update messages on obsoleted members (they're imprecise) Update messages on obsoleted methods #194
• Retrospective (scheduled for Tue 5th Apr)

Related to #100

[mauroservienti]

We decided to drop the multi-instance support for the V3 of the SQL Transport.
Being a major release we could drop the feature without any concern, but we want to provide customers using it an exit strategy. We actually have a working spike that enables the multi-instance support as it was in V2, we are faced with 3 options:

1. Release it as an unsupported sample that explain to customers how to build that feature on their own
2. Release it as a downstream Nuget package that extends the transport configuration with the legacy options
3. Release it as part of the transport itself adding a configuration option, such as an appSetting, to enable it

The last one appears to be the most interesting, cause theoretically it enables the transport to work in ServiceControl as well without any change to ServiceControl. We now know that it is not true, actually ServiceControl crashes when using the SQL transport and the multi-instance due to some expectations in ServiceControl not met by the transport, this raises the issue that the transport may have an opinion on the ServiceControl configuration, if we fix this then option (2) is the best in my opinion since from the user perspective an official package is much better than a sample and from our perspective it may end up having a different support life cycle.

Along with this decision we also strongly think that we should release a set of guidance on why multi-instance is a bad design decision and how to implement shovel-like solutions in a SQL transport based topology.

We think that the async transport task force should only be responsible of:

• making the decision
• listing options to move forward
• spiking any shovel-like implementation
• highlighting ServiceControl issues

[ramonsmits]

@mauroservienti Till now I've only read multi instance is dropped and not multi database but both are not supported I guess?

Lets get our vocabulary straight and use SQL Server terminology:

• Server, The host
• Instance, The SQL process. Can have multple on a single server
• Catalog, The database. Can have multiple on a single instance
• Schema, The schema. Can have multiple in a single catalog

When saying multi database we basicallly say multi catalog is not supported and that indirectly implies multi instance is not supported too.

As I've advocated in #113 I would favor adding catalog and schema identifiers in the queue names. That would make it very easy to support both multi schema and multi catalog while sharing the same / a single connection string. It needs no trickery by defining a schema and/or catalog mapping somewhere as this is the native format of the transport and is readable by any DBA.

What would be wrong with supporting fully qualified names name like [Sales]..[OrderProcessing] ?

Is such a a queue name too coupled to the transport? Is it that we only want to use 'logical' queues and not physical?

[mauroservienti]

@ramonsmits we never had a distinction between multi-instance and multi-database, but I think that could be interesting to support the multi-database scenario.

@tmasternak thoughts?

[ramonsmits]

@mauroservienti No I understand as previously we had the mindset to only access a single database on a sql connection (string) but that is restriction we set ourselves.

[tmasternak]

I think that I makes sense to support this. On a technical level what @ramonsmits proposes should not be a problem. I need to verify if we can do it in backwards compatible way but I think it should work.

@mauroservienti: should I create an issue for that. It can probably be just a spike to figure out if we can do it later for 3.1.

[mauroservienti]

Yes let's create a separate issue and consider it for the next minor.

[tmasternak]

I'm still working on extension point PR. Should be ready this week.

[tmasternak]

This is work in progress. We are working on legacy multi-instance code sample //cc: @justabitofcode

[tmasternak]

I've cleaned up the issue removing the non-core v6 items.

[weralabaj]

Regarding "Documentation/Guidance around security" I think that should be raised as a separate issue in docs. It's not specifically V6 related.

[tmasternak]

We are waiting for doc PR to get merged and this one is done.

[SimonCropp]

@tmasternak can u link to the docs PR

[tmasternak]

@SimonCropp I've added PoC for the docs issue that is already linked (the only outstanding in PoC here)

[mauroservienti]

Will this be included in the retrospective we already scheduled?

[weralabaj]

@mauroservienti Of course, that was part of that bigger task & the same TF, so I think we should do it together.

[weralabaj]

According to today's discussion - I'm reopening this issue for retrospective.

[tmasternak]

Removing the In Progress - this has not been scheduled.
@weralabaj I think that we should close this one and add Consider for Promotion the retrospective one.

[indualagarsamy]

Currently we have users who use the multi-instance / multi-catalog support.
@Particular/sqlserver-transport-maintainers - Please review this case https://nservicebus.desk.com/agent/case/18278 and answer the user.