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

Postgres locking DAO #102

Merged
merged 11 commits into from
Apr 9, 2024
Merged

Postgres locking DAO #102

merged 11 commits into from
Apr 9, 2024

Conversation

VerstraeteBert
Copy link
Contributor

@VerstraeteBert VerstraeteBert commented Mar 7, 2024

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

Changes in this PR

Implement Postgres locking DAO

Fixes the issues observed in #81

A remaining issue is the fact that lock leases may not cover the full time the thread want to hold the lock is covered (e.g., long GCs, ...), this issue is present in the Redis lock implementation as well and would be solved the alternative outlined below.

Alternatives considered

Postgres has row level and table level locks, in addition to application-defined locks through advisory locking. Both of these are on the transaction level, which does not work with the current architecture of Conductor as transaction are handled at the lowest level (i.e., per database operation). To allow this to work, a larger overhaul is required where transaction are managed at the service level.

@bjpirt
Copy link
Contributor

bjpirt commented Mar 10, 2024

@VerstraeteBert Looks like the build is failing because there's already a V9 migration - renaming to V11 should do it

@v1r3n v1r3n merged commit 7e0128d into conductor-oss:main Apr 9, 2024
1 of 2 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.

None yet

4 participants