-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add PostgreSQL Messaging Pub/Sub Watermill #1680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
Maybe a stupid question, but since there appears to be a single table, how does publishing and subscribing work with multiple minder processes? Does the subscriber implement some locking with e.g. SELECT FOR UPDATE? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the postgres driver brings some new tables to the mix. Where are those created? The minder-server
database user does not have permissions to create tables, only the migration user is able to do this. I'm guessing this would be an issue.
3a4d2be
to
ec775c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove the block once we discuss the env variable, but on first glance it seems incorrect
This removes the boilerplate on other parts of the code and instead moves the responsibility to eventer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
Env var will be added as an extraEnv in the mediator-app.yaml in infra
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minder analyzed this PR and found no vulnerable dependencies.
Good question, to answer this one, I did some digging in files like https://github.com/ThreeDotsLabs/watermill-sql/blob/master/pkg/sql/offsets_adapter_postgresql.go and https://github.com/ThreeDotsLabs/watermill/blob/master/_examples/real-world-examples/exactly-once-delivery-counter/README.md, and the scarce documentation in https://watermill.io/pubsubs/sql/ so I can present an overall idea of how it functions: There are essentially two types of tables involved for each topic and consumer group: Message Table:
Offsets Table:
Relationship Between the TablesThe message table and the offsets table are related in the sense that the offsets table keeps track of which messages from the message table have been processed. The offsets table stores an identifier (like an offset or a transaction ID) that corresponds to records in the message table. |
One more question, but I guess this can be a follow-up..how is the InitSchema attribute used? I can't seem to find the usage:
|
It isn't as of now. I made it configurable because I wanna figure out if it's possible to move this logic out of the workload, so then we could just toggle that. |
Should we remove it for now and re-add it when it's present in the application logic? |
Issue described in: #1636
Testing
Update: after adding the second database "minder_queue", we have a nice separation of concerns: