Add BatchEnqueue for pipelined multi-task enqueue#1094
Open
enilsen16 wants to merge 5 commits intohibiken:masterfrom
Open
Add BatchEnqueue for pipelined multi-task enqueue#1094enilsen16 wants to merge 5 commits intohibiken:masterfrom
enilsen16 wants to merge 5 commits intohibiken:masterfrom
Conversation
Adds BatchEnqueue to the Broker interface and RDB implementation that sends multiple enqueueCmd Lua script invocations in a single Redis pipeline round-trip. Also adds BatchEnqueueContext to the Client as the public API, returning per-task results for partial-success handling. Ref: hibiken#1069
BatchEnqueueContext had a time comparison bug where `now` was captured before the loop but `processAt` was set to time.Now() inside composeOptions during each iteration, causing all immediate tasks to be incorrectly classified as scheduled and rejected. Fix: move `now` capture inside the loop, after composeOptions. Additionally, extend BatchEnqueueContext to support scheduled tasks in the same pipeline. Tasks with a future ProcessAt are now routed to scheduleCmd (ZADD to scheduled set) instead of being rejected. Only unique and group tasks remain unsupported. Changes: - Add BatchEnqueueItem type pairing TaskMessage with optional ProcessAt - Update Broker interface, RDB, and testbroker to use BatchEnqueueItem - Route immediate tasks to enqueueCmd, scheduled tasks to scheduleCmd - Return correct TaskState (Pending vs Scheduled) in results - Add tests for immediate, scheduled, and mixed batch scenarios
Exposes the opts field so callers can read a task's options to merge with additional per-task options at batch time.
Collaborator
|
Please document any atomicity guarantees e.g. what kind of failures would or would not reject the entire batch. |
internal/rdb/rdb.go
Outdated
| // single Redis pipeline round-trip. It returns the number of newly enqueued | ||
| // messages (tasks whose IDs already exist in Redis are silently skipped). | ||
| // BatchEnqueue adds all given tasks to Redis using a single pipeline round-trip. | ||
| // Each item is either enqueued immediately or scheduled based on its ProcessAt field. |
Collaborator
There was a problem hiding this comment.
Add a clear warning on silent skipping of duplicates on its own line e.g.
WARNING: tasks whose IDs already exist in Redis are silently skipped
Add comprehensive doc comments to BatchEnqueueContext, the Broker interface method, and the RDB implementation explaining that the batch uses a Redis pipeline (not MULTI/EXEC), so partial success is possible and individual Lua scripts are atomic but the batch is not. Fix a bug where Script.Run inside a pipeline only sends EVALSHA without the automatic EVAL fallback that non-pipeline calls get. On a fresh Redis (or after SCRIPT FLUSH), this caused NOSCRIPT errors for every pipeline-batched script invocation. The fix preloads the required Lua scripts before building the pipeline. Also roll back the in-memory queuesPublished cache when the pipeline fails, preventing stale entries from suppressing future SADD calls.
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.
Adds BatchEnqueue to the Broker interface and RDB implementation that sends multiple enqueueCmd Lua script invocations in a single Redis pipeline round-trip. Also adds BatchEnqueueContext to the Client as the public API, returning per-task results for partial-success handling.
Ref: #1069