fix: avoid collisions in auto-generated Job IDs causing dropped jobs#237
fix: avoid collisions in auto-generated Job IDs causing dropped jobs#237hughsw wants to merge 5 commits intobee-queue:masterfrom
Conversation
|
[ This issue is resolved ] This approach breaks the semantic that custom job id's don't affect The semantic could be implemented, but:
|
|
Actually, it's not hard to preserve the existing semantic, but I'd like some discussion about the necessity of this semantic and the test. |
3de0d6d to
20d411e
Compare
|
[ This issue is resolved for Wrapping |
|
So the one remaining test failure is baffling to me. The deepEqual of two Set object fails because the items in the sets are in different orders (even though the set of items is the same). It is reasonable that the code changes for uuid Job.id would cause the ordering to have changed, but my understanding is that Sets should be deepEqual regardless of the order. Any insight into the failure? If I sort the error arrays (and deepEqual the arrays rather than going to the trouble of making Sets), the test passes, so that's likely what I'm going to add to this PR. Here's a typical failure I get locally (similar to Travis failures): |
|
I'll try and take a look later tomorrow. I'm also curious what you think of #232. |
5470e0e to
f6a1d1a
Compare
compwright
left a comment
There was a problem hiding this comment.
Approved pending rebase
| @@ -1,5 +1,5 @@ | |||
| --[[ | |||
| key 1 -> bq:name:id (job ID counter) | |||
| key 1 -> bq:name:id (job counter) | |||
There was a problem hiding this comment.
What is stored in bq:name:id? My understanding from this PR is that it's the number of jobs that don't have a custom id? What's that used for?
There was a problem hiding this comment.
I believe that is correct, it's poorly named, and appears to come from the fact that auto-generated IDs are currently sequential. So job ID 1000 would be the 1000th job. This PR of course changes that but does not rename to try to maximize compatibility, replacing the stat with the actual count instead of the actual last job ID from which you would infer the count.
There was a problem hiding this comment.
Where is that information ("the number of jobs that don't have a custom id") used?
Alternate fix for #78 and #189
Builds on #193 , returning to its original intent
Uses random hex string for Job.id, generated and registered in Queue prior to adding job to Redis.