-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Host.Outbox] Support for UUIDv7, different message keys and refactor #315
Conversation
src/SlimMessageBus.Host.Outbox/Interceptors/OutboxForwardingPublishInterceptor.cs
Outdated
Show resolved
Hide resolved
@dundich here is another iteration:
There is some more work needed to
|
Quality Gate passedIssues Measures |
I agree it is worth defining the API for connecting Outbox strategies. But before Api I think it is necessary to define possible interception points for working with OutboxFlow. So far I see them in three places: Interception points
They should probably be set up through configuration. Hybrid outbox ?like as (https://github.com/zarusz/SlimMessageBus/blob/master/docs/provider_hybrid.md) for (mssql, pg, redis) I mean connect multiple strategies and then bind them via configuration but imho overhead - maybe it's not worth it yet and with an eye to the future.? Describe the prototype connection APIlike as #282 (comment) note: I'm sorry I can't look at the code yet.. ps Perhaps then it will become clearer whether we need additional abstractions. |
@dundich all great ideas, but that needs to be done in stages. The intent of this PR is to introduce a way to inject the Guid generator and connect the existing time provider interface into outbox. This is to enable your need for UUIDv7. Any further enhancements need to come after that. Next stages could include:
Let me know if you have other priorities in mind. |
As for the hybrid outbox my thinking is this:
Also, the idea with Not sure that you'd need a hybrid outbox with both some messages routed to SQL outbox, others to Redis Outbox. Again, we need to do this in stages :) |
I agree with all of this - the elephant needs to be eaten in pieces. As for the dialect, I didn't quite catch the point. I thought it was enough to register my IPgSqlRepository implementation via config... and why I need a dialect, I don't understand yet..... it seems to me that you want to put everything in one SlimMessageBus.Host.Outbox.Sql or am I wrong? ps. To get started on Pg I don't like Mssql yet. Do I understand correctly that we get rid of it, leaving IConnection ITransaction abstractions without vendors? |
Oh yes I see we would need a new project |
3ee000f
to
f0a2799
Compare
82ef2b8
to
2a52846
Compare
@dundich I am ready with all my changes. Hopefully, this will give a baseline for your Postgress Outbox work (although some more refactoring might be needed). Certainly, it will allow for Guid generation strategies e.g. UUIDv7. |
Refactoring - need to understand how best to reorganize two projects with mssql:
ps. |
note: There is one point - since the contract has been rewritten, now id generation may come with a response from Pg or Mssql, the generator may be useless here (uid_generate_v1mc() or NEWSEQUENTIALID (Transact-SQL) orderable by time - guid ). but he'll still be useful for nosql ... or saga in future |
I think at this point I would just start with a new project (e.g. SlimMessageBus.Host.Outbox.PgSql or Outbox.PostgreSql) and see how it goes. Ideally if you can keep is such that The
Yes, that is true. I wasn't aware of the new NEWSEQUENTIALID. Makes sense to also have that as an option on the Outbox.Sql. Let me see if perhaps this is worth adding to my PR! Then on the postgree side you could adopt a similar approach/setting. |
328add6
to
eb6bd33
Compare
@dundich I am debating whether not to refactor this
There are still a few considerations around managing transactions
The only caveat is if we let EF Core to manage the outbox messages it might end up being slower than the current T-SQL implementation. cc: @EtherZa (in case wanted to weigh in) |
@zarusz I would be apprehensive to create a single plugin that supports multiple database providers. There are slight variations in the dialects that would result in a number of conditionals (non-clustered indexes in sql but not in postgres or mysql, select top vs limit, etc). Further to this, there are features that can be leveraged in some providers that are not available in others (postgres notficiations). As such, you will get a far better implementation with a tailored solution. As for using EF; you run the risk of pinning the EF version which would be a deal breaker for me personally. It would also reduce the feature set to the lowest common denominator. I agree with @dundich that the implementation/DTO could be cleaned up. The provider specific implementation can be hidden from As a side note (and definitely scope creep for this discussion); the current implementation could be vastly improved if the message processing were to be partitioned by path (and subscription for ASB). Processing by partition (round-robin, instead of purely by sequence) would ensure that multiple queues are populated instead of just one working through a backlog. ie. In the workflow A => B, where A enqueues a message for B; if 100k messages are outboxed for the A queue, the current implementation will sequentially run through the messages adding all 100k to A before starting to add anything to the B queue. The B consumers then sit idle while A works its way through the backlog. I only bring this up as it could be benefitial to keep in mind if refactoring is planned. |
I would like to join right now, but I am busy and so I will keep it short I am also against linking to EF. I want to use PG features such as: partitions, skip-locked I would focus on contract to connect different implementations and customize them.
ps. then connect MSSQL (as a template for others) |
Again, I think the problem lies in the name - repository. It is clear that a repository should be as simple as possible, hence the desire to have one BLL with single settings. I think this is a mistake. We should allow each vendor to have their own properties and define Outbox logic independently, just like Buses have their own specifics.. e.g. the vendor's use of 3NF or lowcardinality for the fields: BusName , MessageType, Path, InstanceId |
@dundich and @EtherZa thanks for your perspectives and ideas. The idea with EF Core serving as an abstraction would be an addition to ensure broader databases coverage. So let say we would still have a MSSQL (like today) and PG specific plugin (hopefully built by @dundich), but any other db would fallback to a slower EF implementation. I share the same over abstraction and lower performance concern in contrast when comparing EF approach vs optimized Outbox.Sql or PQ. Existing ones that we'd preserve in v3.0.0
The new ones I can foresee:
@EtherZa the partitioning feature on MSSQL is a good one, I will capture as another feature request. @dundich again, the repository is a way to delegate the db specific SQL dialect (e.g. Outbox.Sql / Outbox.Pg) and hopefully for the db specialized plugins to still leverage the common logic within Host.Outbox. We could rename to OutboxService or something if that sits better. Now, we tried to move the PK out of the entity to allow for provider specific Key types, I could still try to revive that. My stance on the abstraction in |
Rather, you're right in that the DBs have common specifics. Please consider a few points:
|
a9238c9
to
87bb9cf
Compare
@dundich, I think I am now done with my changes.
Happy to merge, unless you see anything out of line. |
pls look at small fix for uuidv7 - #325 |
Yes, good idea! Merged. |
888401d
to
19fed04
Compare
… flexible PK types, refactor Signed-off-by: Tomasz Maruszak <maruszaktomasz@gmail.com>
c492eb2
to
990f1f2
Compare
Several changes to
SlimMessageBus.Host.Outbox.*
in preparation for onboarding other storage providers:IGuidGenerator
inSlimMessageBus.Host
GuidGenerator
registered in DI.Outbox.Sql
layer the ability in theSqlOutboxSettings
toGuidGeneratorType
in the MSDI orGuidGenerator
instance for the client ID generation.SlimMessageBus.Host.Outbox.DbContext
toSlimMessageBus.Host.Outbox.Sql.DbContext
- to express its bound to MSSQLOutboxMessage
down to the .Outbox.Sql layer into a specialized typeSqlOutboxMessage
..Outbox.Sql
will still use theGuid
for backward compatibilityCloses #311