Particular / ServiceControl

Backend for ServiceInsight and ServicePulse
https://docs.particular.net/servicecontrol/
Other
51 stars 47 forks source link

Allow transports to have an opinion on ServiceControl configuration at startup time #618

Closed mauroservienti closed 8 years ago

mauroservienti commented 8 years ago

ServiceControl has a strong opinion on the transaction configuration and at bus configuration time it disables support for DTC transactions. There are scenarios, when using different transports, that this can lead to a situation where ServiceControl is unusable (see #609), one of these scenarios is caused by the SQL Transport when the multi-database/instance feature is enabled. The transport expects to have support for the DTC or for the Outbox in such a scenario and since none of the requirements is met by ServiceControl the transport crashes the host.

Would be interesting to allow a transport to have an opinion on the ServiceControl bus configuration, this can actually be achieved using the following hack, e.g. to enable DTC support:

    public class ServiceControlDTCSupportFix : NServiceBus.INeedInitialization
    {
        public void Customize(BusConfiguration configuration)
        {
            configuration.Transactions()
                .EnableDistributedTransactions()
                .WrapHandlersExecutionInATransactionScope();
        }
    }

or this one to enable the Outbox (plus the configuration setting in the ServiceControl config file)

    public class ServiceControlEnableOutboxFix : NServiceBus.INeedInitialization
    {
        public void Customize(BusConfiguration configuration)
        {
            configuration.EnableOutbox();
        }
    }

Dropping an assembly with the code above in the ServiceControl bin folder and restarting ServiceControl, cause ServiceControl to automatically pick up the INeedInitialization instance and execute it as part of the configuration process.

Both the above approaches solve the aforementioned issue but are still a hack since there is no control in Core on the execution order of INeedInitialization instances, meaning that another INeedInitialization instance could be executed after the one above changing again the ServiceControl configuration.

ServiceControl should have a dedicated way to allow the transport, and the transport only, to have an opinion on the configuration.

This should not be part of the transport package itself but should be delivered as a separate package for ServiceControl installation only.

This package should also be able to plug logic into the ServiceControl management UI and into the ServiceControl instance unattended installation.

Please @Particular/tf-async-transport and @Particular/servicecontrol chime in.

gbiellem commented 8 years ago

@mauroservienti there is also a third setup mechanism, we have some PS cmdlets so they would also need to be pluggable.

What is wrong with just just making these options configurable? I'm not sold that the installation need to be pluggable to achieve that. Seems like over engineering

mauroservienti commented 8 years ago

Transports may have multiple, different and complex requirements I'd prefer to avoid to clutter ServiceControl configuration with transports details like we do now with RavenDB settings. This would cause ServiceControl to run after transports and to duplicate logic and code in ServiceControl to expose what a transport may already expose. For example in the new version of the SQL transport we are planning to allow the user be user to plugin their own logic to define the multi-instance topology and to distribute connection strings, a user doing this in their system must be able to configure ServiceControl in the same way, and it is more complex than a setting in a configuration file. ASB has the same issue with multiple supported topologies.

But extending it is a solution to the specific highlighted issue.

Mauro Servienti | Solution Architect | Particular Software

mauroservienti commented 8 years ago

Another issue in adding settings, that are indeed transport related, to ServiceControl is that not all the settings may have a meaning for all the transports, forcing us to document all the valid combinations based on the chosen transport. E.g. for ASB enabling the DTC has no meaning at all, but the outbox could be an interesting addition.

gbiellem commented 8 years ago

For example in the new version of the SQL transport we are planning to allow the user be user to plugin their own logic to define the multi-instance topology and to distribute connection strings.

How do you see this working in practice? Do you see the SC installation prompting for a custom DLL at the time the instance in installed?

mauroservienti commented 8 years ago

No, I expect it to work by convention or by DI. E.g. I expect that ServiceControl exposes a contract such as ICustomizeServiceControlConfiguration and at startup time, configuration time via the management UI or via PS or unattended installation, an implementation of that interface is automatically picked up, exactly like we do with INeedInitialization, a transport can then supply an implementation of the above contract and express its opinion.

Obviously the above solves only the issue of how to automate the configuration customization by the transport, but does not allow any interactive or unattended customization by Ops people. it is not its responsibility. What I envision is something similar to what a visual editor does when dealing with visual controls, e.g. Visual Studio. Basic properties can be automatically edited without any intervention via the property grid but for custom advanced editing of complex properties a custom editor can be supplied by the control itself. In a similar fashion what I expect is that a transport, if needed, can provide to ServiceControl:

Must all the above be provided? it's a transport decision, I'd say should.

It is not that different from the same experience you make when configuring a connection string to a database, in some scenarios (e.g. ODBC or Oracle) the connection string builder UI is provided by the database driver and it is not the default system one.

johnsimons commented 8 years ago

I am still on the fence on this one, if we design an idempotent system, we should not need either outbox or dtc on.

@andreasohlund thoughts about this issue ?

mauroservienti commented 8 years ago

What it is not yet clear to me is how it is possible that a system, ServiceControl, that deals with at least 2 resources, can work in a exactly once delivery fashion without using the DTC.

Is there any reference code you can point me to to make me understand how retries are designed to make them work using a exactly once semantic?

johnsimons commented 8 years ago

We use the DB to dedup messages

johnsimons commented 8 years ago

DB = Raven documents

tmasternak commented 8 years ago

@mauroservienti I like the idea of customization. One question I would ask though is "Are Ops the people that have enough knowledge about the system to make transport configuration decisions?". Maybe there exists "the safest configuration" for each transport that should be used. That said even with SqlTransport it seems that sometimes DTC vs Outbox decision will have to be made. I'm not sure if that will change SC configuration workflow/mechanism but it might.

@johnsimons You mean RavenDB hosted by SC right?

andreasohlund commented 8 years ago

I think this issue highlights why @danielmarbach proposed to decouple SC from the transports since that would allow us to run SC with a known consistency model and all the transport details would be hidden behind our "transport anti corruption layer". Ie if SC needs messaging internally we would always use the same messaging infra regardless of what transport the user is currently running. This would allow us to support users running multiple transports as well.

tmasternak commented 8 years ago

@mauroservienti @andreasohlund After talking with @weralabaj I think that we can look at this as two step process:

johnsimons commented 8 years ago

We are adding a hack to support the most critical issue we have so far regarding the multi-catalog feature in sql transport. Everything else i think once we start to look at Platform Componentization we can look at this more seriously and reevaluate if the way we are currently using the transport implementations is correct or not. So for now we put this one to sleep.