Skip to content
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

Support NpgsqlMultiHostDataSource #3112

Merged
merged 1 commit into from
May 8, 2024

Conversation

Hawxy
Copy link
Contributor

@Hawxy Hawxy commented Apr 3, 2024

Requires JasperFx/weasel#126

This is a draft for the time being to see if the design is suitable. I've tried to achieve this with the least amount of code changes possible. Docs yet to be written.
Some considerations:

  • NpgsqlMultiHostDataSource registers itself in the DI container as a normal data source, so marten won't explicitly know it's a multi host source until it tries to fetch a connection from it. If marten has no defaults for multi host usage then the data source will return any connection type and potentially cause unexpected errors as it tries to write against a read replica. For this reason, I've baked in some sane fallbacks for marten & weasel.
  • If a NpgsqlMultiHostDataSource is present we won't target the replica by default, it's up to the user to weigh up the potential pitfalls of replica lag.
  • I'm only targeting user queries for this functionality. All internal queries including daemon operation will remain pointed at the primary node for reliability. (although for distributed projections/rebuilds there might be performance benefits to using the replicas)
  • Does Wolverine have support for a user-supplied data source yet? The fix for Wolverine.Postgres is just to get users to only supply a datasource capable of creating primary node connections dataSource.WithTargetSession(TargetSessionAttributes.Primary);
  • This implementation only supports the AutoClosingLifetime. I'm unsure if it's practical to support the others just yet.

@jeremydmiller
Copy link
Member

I we're good to go with Wolverine. It's able to just copy over the Marten data source when it can (that hurt getting there and I was whiney about Npgsql on Twitter when that happened).

@jeremydmiller
Copy link
Member

@Hawxy Sorry this has taken so long. I'm good with that as is. I can do the work to pull over the Weasel changes any time, or I can make sure you have all the rights if you wanna do that

@Hawxy Hawxy force-pushed the npgsqlmultihost branch from 1f2814c to ec54811 Compare May 7, 2024 12:20
@Hawxy Hawxy marked this pull request as ready for review May 7, 2024 12:20
@Hawxy Hawxy force-pushed the npgsqlmultihost branch from 7a11456 to 806dfe3 Compare May 7, 2024 12:23
@jeremydmiller jeremydmiller merged commit c4f8294 into JasperFx:master May 8, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants