Replace lib-data-stream-redis with lib-data-workqueue - #1115
Open
pditommaso wants to merge 1 commit into
Open
Conversation
Move the job queues from the message stream API onto the work queue API of libseqera. `lib-data-workqueue-redis` is `lib-data-stream-redis` 1.5.0 on lease semantics: the same Redis stream layout (xadd/xreadGroup/xautoclaim, xack+xdel on settle, same `data` field), so the queue keys and the consumer group are left untouched and messages queued before the upgrade are still delivered afterwards. What the lease buys over the previous claim-timeout heuristic: an in-flight entry is renewed at visibility-timeout/4, so a slow consumer's message can no longer be claimed by another replica mid-processing; and close() waits cooperatively for the current cycle instead of interrupting the listener thread after 1s, letting a consumer finish its writes before teardown. `JobManager` keeps its own admission control - launchJob/processJob are unchanged and still return a boolean, now mapped onto ACK/RETRY at the addConsumer site - so the maxRunningJobs cap on concurrently running jobs is unaffected. Config keys are renamed `wave.message-stream.*` -> `wave.work-queue.*` (claim-timeout -> visibility-timeout). The consumer group keeps defaulting to `wave-message-stream` so existing pending entries stay visible. The prod/stage overrides in platform-deployment need the matching rename. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Moves the job queues from the message stream API onto the work queue API of libseqera:
lib-data-stream-redis:1.2.0→lib-data-workqueue:2.0.0+lib-data-workqueue-redis:2.0.0.lib-data-workqueue-redisislib-data-stream-redis1.5.0 re-landed on lease semantics — identical Redis stream layout (xadd/xreadGroup/xautoclaim,xack+xdelon settle, samedatafield). Queue keys (jobs-pending/v2,jobs-queue/v1) and the consumer group are left untouched, so no migration is needed and messages queued before the upgrade are still delivered afterwards.Changes
build.gradleservice/data/workqueue/BaseWorkQueueBaseMessageStream, now onAbstractWorkQueue; same Moshi encoding strategyservice/data/workqueue/RedisWorkQueueConfigBeanRedisStreamConfigBean, now implementsRedisWorkQueueConfigJobPendingQueue/JobProcessingQueueBaseWorkQueue; queue ids unchangedJobManager(job) -> booleanbecomes(job, lease) -> Decision.ACK/RETRYdocs/configuration.mdlaunchJob/processJobare unchanged and still return a boolean, mapped ontoACK/RETRYat theaddConsumersite — soJobManagerkeeps its own admission control and themaxRunningJobscap is unaffected.Why: work queue vs message stream
claim-timeoutcan be claimed and run concurrently by another replica. The timeout had to exceed worst-case processing time — hence 45s in prod.visibility-timeout/4while the owner is alive (oneXCLAIM JUSTIDper queue per tick), holding the idle clock near zero. The timeout now only governs recovery from a dead owner.thread.interrupt()thenjoin(1s)— severs a consumer mid-processing, and an interrupt can hand a RESP-desynced connection back to the Jedis pool.closingflag checked at loop head, waits (default 10s) for the current cycle, never interrupts. AprocessJobcycle completes itsnotifyJobCompletion/cleanupbefore teardown.awaitQuiescent(timeout)available for a bounded drain.falseskipped the poll-interval sleep.ACK/DEFERREDcount as progress, so a queue that is merely retrying (job still running, pending queue full) paces at its poll interval.XPENDINGownership check first: an entry that drifted to another consumer is dropped and logged rather than seized back — the residual duplicate window is observable, not silent.Not used by this PR, but now available:
MessageConsumer.ready()(an admission gate checked before claiming — the natural home for themaxRunningJobscheck, which currently claims a message and then refuses it),MessageLease/DEFERREDwithretryAfter(delay), and lease/renewal/saturation metrics. Still at-least-once: renewal narrows the duplicate window, it does not close it.Rollout
No Redis migration. Entries queued by the old code are read as-is (the integration test logs
consume group=wave-message-stream already exists), and entries left pending by a terminating pod are reclaimed after the visibility timeout — in-flight jobs resume. Mixed-version replicas are safe both ways: an entry held by a new replica is renewed so an old replica's 45s-min-idle claim never sees it; an entry held by an old replica may be claimed after 45s idle, exactly as today.Config keys are renamed
wave.message-stream.*→wave.work-queue.*(claim-timeout→visibility-timeout), so the matching rename in platform-deployment (prod + stage,45s) must land with this. That change is prepared but not yet pushed.message-stream.claim-timeout, falling back to a 5s claim timeout with no renewal. Keeping both key sets in the ConfigMap for one release makes it order-independent in both directions. Running pods are unaffected either way — the ConfigMap is mounted viasubPath(never updated in place) and the Deployment has no config checksum annotation, so config is only read at container start.Also note the cooperative
close()can take up to 10s per queue, sequentially across two beans, against the default 30sterminationGracePeriodSeconds. In practice the dispatcher exits at its next loop-head check unless mid-consume; raising the grace period would add headroom.The consumer group keeps defaulting to
wave-message-streamdeliberately — it is a wire identifier now, and renaming it would strand the pending-entry list.Testing
compileGroovycleanio.seqera.wave.service.job.*— 39 tests, 0 failuresBuildStoreRedisTest(7) andRegistryControllerRedisTest(2) green against real Redis; logs confirmRedisWorkQueuereusing the existing consumer group and picking up the config bean (lease renewal period=1250ms)Full suite not run locally — left to CI.
🤖 Generated with Claude Code