Merged
Conversation
bgentry
approved these changes
Aug 28, 2024
Contributor
bgentry
left a comment
There was a problem hiding this comment.
Nice! While you're working on this, would you mind taking a look at bumping rexml to clear up all these dependabot & security scan alerts? https://github.com/riverqueue/riverqueue-ruby/security/dependabot
CHANGELOG.md
Outdated
|
|
||
| ### Changed | ||
|
|
||
| - Now compatible with "fast path" unique job insertion that uses a unique index instead of advisory lock and fetch [as introduced in River #451](https://github.com/riverqueue/river/pull/451). [PR #XXX](https://github.com/riverqueue/riverqueue-ruby/pull/XXX). |
Contributor
There was a problem hiding this comment.
Suggested change
| - Now compatible with "fast path" unique job insertion that uses a unique index instead of advisory lock and fetch [as introduced in River #451](https://github.com/riverqueue/river/pull/451). [PR #XXX](https://github.com/riverqueue/riverqueue-ruby/pull/XXX). | |
| - Now compatible with "fast path" unique job insertion that uses a unique index instead of advisory lock and fetch [as introduced in River #451](https://github.com/riverqueue/river/pull/451). [PR #28](https://github.com/riverqueue/riverqueue-ruby/pull/28). |
Comment on lines
67
to
72
| # It'd be nice to specify this as `(kind, unique_key) WHERE unique_key | ||
| # IS NOT NULL` like we do elsewhere, but in its pure ingenuity, fucking | ||
| # ActiveRecord tries to look up a unique index instead of letting | ||
| # Postgres handle that, and of course it doesn't support a `WHERE` | ||
| # clause. | ||
| unique_by: "river_job_kind_unique_key_idx" |
Contributor
Author
There was a problem hiding this comment.
Haha, felt a little hard done by the time I finally found something that worked.
5e0e9a2 to
eda0e4e
Compare
Contributor
Author
Yep! It's been a while since I bumped the gems in this project. Let me tackle that right after. |
eda0e4e to
6e75e81
Compare
Updates the Ruby client to be compatible with the fast unique insertion added to the main River in [1] which uses a unique index instead of advisory lock + fetch as long as uniqueness is constrained to the default set of unique job states. We also reorganize the driver tests such that the majority of the tests are put in a single set of shared examples, largely so that ActiveRecord and Sequel aren't so duplicative of each other, and so we can easily add new tests for all drivers in one place. Lastly, I killed the mock driver in use at the top level. Adding anything new required all kinds of engineering around it, and I found lots of test bugs that were the result of imperfect mocking that wasn't fully checking the client end to end. [1] riverqueue/river#451
6e75e81 to
cab6dd4
Compare
brandur
added a commit
that referenced
this pull request
Aug 31, 2024
Prepare v0.7.0, containing largely the changes in #28 that add support for past path unique job insertion.
Merged
brandur
added a commit
that referenced
this pull request
Aug 31, 2024
Prepare v0.7.0, containing largely the changes in #28 that add support for past path unique job insertion.
brandur
added a commit
that referenced
this pull request
Feb 27, 2025
Remove a few straggler references saying that advisory locks provide uniqueness. As of #28, this is no longer true and a unique index is used instead.
brandur
added a commit
that referenced
this pull request
Feb 28, 2025
Remove a few straggler references saying that advisory locks provide uniqueness. As of #28, this is no longer true and a unique index is used instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates the Ruby client to be compatible with the fast unique insertion
added to the main River in [1] which uses a unique index instead of
advisory lock + fetch as long as uniqueness is constrained to the
default set of unique job states.
We also reorganize the driver tests such that the majority of the tests
are put in a single set of shared examples, largely so that ActiveRecord
and Sequel aren't so duplicative of each other, and so we can easily add
new tests for all drivers in one place.
Lastly, I killed the mock driver in use at the top level. Adding
anything new required all kinds of engineering around it, and I found
lots of test bugs that were the result of imperfect mocking that wasn't
fully checking the client end to end.
[1] riverqueue/river#451