Revert lib-data-stream-redis to 1.5.0 and lib-cmd-queue-redis to 0.4.0 - #100
Conversation
Restore both module trees to their state at 8bad0f5 ([release] lib-data-stream-redis@1.5.0, 14 May 2026) — the commit where stream was 1.5.0 and cmd-queue 0.4.0 — undoing everything released on top of them: - stream 2.0.0 / cmd-queue 0.5.0: async, non-blocking consumer processing with heartbeat lease and the poll/renew/ack/release SPI (PR #84, bdf9374), plus doc follow-ups 11fdd3a and 86ae9ac - cmd-queue 0.5.1: retry command on handler exception (PR #87, 6c7b171) - cmd-queue 0.6.0: error tracking on CommandState (PR #89, e54229e, 1f124f0) - cmd-queue 0.7.0: migration to lib-data-workqueue(-redis) and the CommandStatus SUBMITTED->PENDING / RUNNING->PROCESSING rename (PR #86, f3f4ac4); README follow-up PR #91 (e8fe916) - the parts of PR #94 (d38afb0) that touched these two modules' sources cmd-queue therefore depends on lib-data-stream-redis again. lib-data-workqueue and lib-data-workqueue-redis are left in place untouched — they carry the lease-based design forward and were never published. Two build-infra bits from PR #94 are deliberately kept rather than reverted, since they are repo-wide conventions and not module API: the io.seqera.micronaut-library-conventions plugin id (Java 25 target for Micronaut modules) and Groovy 4.0.31 for stream's test dependencies (4.0.24 cannot run on a JDK 25 toolchain). The changelogs keep a REVERTED entry recording the withdrawn versions with their PRs, and cmd-queue's notes the downgrade hazard: 0.7.0-persisted command state uses PENDING/PROCESSING with a 7-day TTL, which this 0.4.0 code cannot decode. Pre-revert state is preserved on branch archive/workqueue-pre-revert (2ecc744), which also carries the unmerged invocation-lease rework. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| // Unexpected exception during processing - mark as FAILED | ||
| log.error("Command processing failed: id={}", msg.commandId(), e); | ||
| store.save(state.failed(e.getMessage())); | ||
| return true; // Remove from queue - no point retrying a crashed handler |
There was a problem hiding this comment.
Not blocking — the revert is correct as it stands and should land. This is a follow-up request, not a change to this PR.
These three lines are what 6c7b171 (#87) replaced with log; return false, and that commit was written for sched#888's mechanism 14 days before the incident: Hikari pool closed during shutdown, handler throws, Redis state write still succeeds, command acked terminally, task stranded in PENDING forever (sched#712).
It is also separable from #84's async model, which is what sched#772 actually rejected. On 0.4.0/1.5.0 the false return means consume() skips xack/xdel, so the entry stays in the PEL for XAUTOCLAIM to hand to a live pod — sched#888's proposed primary outcome. And because this catch doesn't persist started() (unlike the timeout and explicit-RUNNING paths), status stays SUBMITTED and the next delivery re-enters execute(). That is the "re-run execute() later" primitive sched#889 §1 says 0.4.0 lacks — true of the return-running() path, not of the throw path with #87 applied.
So the ask is: land this, then cut a 0.4.1 = 0.4.0 + this one hunk from this same tree. sched then fixes #888 with a version bump. Java target checked: sched-app is on 25, so a republish from this tree loads fine.
Separately, #87 is worth naming in the risk section rather than only in the undone table — sched#772 re-accepted this regression on the basis that ReconcileCron would re-drive stranded tasks, and sched#889 shows 6d36bad0b never landed on master.
| - 0.6.0 - error tracking on CommandState: errorsCount, error, modifiedAt (PR #89, e54229e) | ||
| - 0.7.0 - migrate to lib-data-workqueue(-redis); CommandStatus SUBMITTED -> PENDING and | ||
| RUNNING -> PROCESSING (PR #86, f3f4ac4) | ||
| - DOWNGRADE WARNING: 0.7.0 persists command state with the renamed PENDING/PROCESSING values |
There was a problem hiding this comment.
Verified this has no live trigger, which is worth saying here so it doesn't read as a rollout prerequisite: sched's pin went 0.5.1 → 0.6.0 → 0.4.0 (a39502274), so 0.7.0 was never deployed, and no other repo pins this module (platform/wave use lib-data-stream-redis 1.1.1/1.1.2).
The direction that did happen — 0.6.0 → 0.4.0 on 22 Jul — is safe and worth recording: JacksonEncodingStrategy disables FAIL_ON_UNKNOWN_PROPERTIES, so 0.6.0's errorsCount / error / modifiedAt are ignored. Right now a reader can't tell whether the downgrade they already performed was OK.
Small precision on the symptom: PROCESSING throws in store.findById() at processCommand step 1, outside the try/catch. AbstractMessageStream.consumeOne catches Throwable, so the consumer survives, but the entry is never acked — it re-claims and re-throws every claim-timeout until the 7-day TTL drops the key. Poison entry with an ERROR loop rather than a silent skip; self-heals at TTL, so the advice itself is right.
| non-blocking consumer processing with a heartbeat lease and the poll/renew/ack/release SPI | ||
| (PR #84, commit bdf9374), plus doc-only follow-ups (11fdd3a, 86ae9ac) and the injected | ||
| handler executor from PR #94 (d38afb0). | ||
| - The lease-based design is not lost: it lives on in lib-data-workqueue / |
There was a problem hiding this comment.
Confirmed self-contained — only settings.gradle:41-42 and lib-data-workqueue-redis/build.gradle:30 reference them, no cross-package imports.
Two stale bits now that nothing consumes them: VERSION is 1.0.0 and lib-data-workqueue-redis/README.md:20-21 tells readers to depend on io.seqera:lib-data-workqueue:1.0.0, which was never published; and that README plus docs/superpowers/specs/2026-07-11-workqueue-rename-migration.md describe these as the split of stream 1.6.0/2.0.0, versions this PR removes from the tree. A one-line "parked, not published" banner would save the next reader.
(Nit: the release list is in .github/workflows/build.yml, not publish.sh.)
… modules Review feedback on #100. No source or test file changes: both reverted module trees remain byte-identical to 8bad0f5. Corrections to the cmd-queue REVERTED note, which overstated the downgrade risk: - 0.7.0 was never deployed. sched pinned 0.5.1 -> 0.6.0 -> 0.4.0 (a39502274, seqeralabs/sched#772) and no other repo depends on this module, so the hazard has no live trigger. Recorded so it does not read as a rollout prerequisite. - The downgrade that did happen, 0.6.0 -> 0.4.0, is safe: JacksonEncodingStrategy disables FAIL_ON_UNKNOWN_PROPERTIES, so 0.6.0's errorsCount / error / modifiedAt are ignored on read. - Fixed the symptom description. PROCESSING throws in store.findById() at step 1 of processCommand, outside the try/catch, so the entry is never acked: the listener survives but the entry re-claims and re-throws every claim-timeout — a poison entry in an ERROR loop, not a silent skip. Self-heals at the 7-day TTL. - Flagged 0.5.1 (#87) as a fix that must be re-landed rather than leaving it in the undone list as if it were part of the rejected async model. It is separable: returning false parks the entry in the PEL for XAUTOCLAIM, and the catch does not persist started(), so status stays SUBMITTED and the next delivery re-enters execute(). lib-data-workqueue / lib-data-workqueue-redis are now parked — nothing consumes them once cmd-queue depends on lib-data-stream-redis again. Both READMEs carried a "migrating from lib-data-stream-redis 1.6.0" callout for a version this PR removes from the tree, and pointed readers at io.seqera:lib-data-workqueue:1.0.0, which was never published. Replaced with a parked/not-published banner, and marked the migration spec HISTORICAL for the same reason. Also corrected in the stream note: this module is no longer deprecated in favour of the workqueue modules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
a7b130d to
7f20ac9
Compare
Restores the fix originally released as 0.5.1 (6c7b171, #87) and dropped by the revert to 0.4.0 in #100. Same hunk, re-cut on top of the 0.4.0 tree. The catch in CommandServiceImpl.processCommandWithHandler treated any escaping exception as a terminal command outcome: it persisted a FAILED CommandState and returned true, so RedisMessageStream.consume acked and deleted the entry. There was no retry. That conflates "the handler threw" with "the command failed", and it fails asymmetrically because the two live in different stores. Command state is in Redis; the domain work is in Postgres. When Micronaut closes the HikariCP pool while the queue is still draining, the handler throws a JDBC error and the catch records a permanent verdict using the store that still works, about a failure caused by the store that does not — while the domain entity was never transitioned. Queue empty, command FAILED, entity dangling, nothing left to advance it, polling clients hanging (seqeralabs/sched#712). Fix: log and return false. The entry stays unacked in the PEL for XAUTOCLAIM to hand to a live consumer, and since the catch never persists started(), status stays SUBMITTED so the next delivery re-enters execute(). A genuine failure is signalled by returning a FAILED CommandResult, which the terminal branch above already handles. Independent of the async/heartbeat-lease model that #100 removed: this works on the synchronous 1.5.0 stream because consume() only xacks/xdels when the consumer returns true. Unchanged: a returned FAILED CommandResult is still terminal, and an unknown command type is still failed and acked (no handler exists to retry) — both covered by existing tests. Test: the spec from #87, verified to fail against the 0.4.0 catch and pass with the fix. Module suite 16/16 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recovers the error tracking originally released as 0.6.0 (e54229e, #89), dropped by the revert to 0.4.0 in #100. VERSION is deliberately untouched. Companion to the retry fix in #102: once a thrown handler is retried instead of terminal-failed, a command can retry indefinitely with nothing recording that it is happening. These fields make that visible. - errorsCount: consecutive processing errors since the last successful processing - modifiedAt: last-write timestamp - error: now also carries the message of a transient (non-terminal) processing error. It holds the most recent message, transient or terminal; a terminal failure is identified by status == FAILED, not by error being non-null. recordError is best-effort — a failed write is logged and never changes control flow, so the command is still kept in the queue and retried. The streak is reset on recovery, with a single write and only when there is something to reset, so healthy re-polls stay write-free. Backward-compatible: the new fields default to 0/null when older serialized state is read. One deliberate adaptation from e54229e, required by this tree: 0.4.x still has executeWithTimeout, which wraps a handler exception in a generic RuntimeException("Command execution failed"). #89 was written against #84, which had removed that method, so recording e.getMessage() verbatim was correct there but here would stamp every transient error on the execute() path with the same useless string. recordError now records the root cause's message via rootMessage(), which is also correct for the checkStatus() path where the exception propagates directly. Caught by #89's own test asserting error == 'Persistent boom'; it failed with 'Command execution failed' before the adaptation. Tests: the two specs from #89 plus its CommandState serialization coverage. Module suite 18/18 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recovers the error tracking originally released as 0.6.0 (e54229e, #89), dropped by the revert to 0.4.0 in #100. VERSION is deliberately untouched. Companion to the retry fix in #102: once a thrown handler is retried instead of terminal-failed, a command can retry indefinitely with nothing recording that it is happening. These fields make that visible. - errorsCount: consecutive processing errors since the last successful processing - modifiedAt: last-write timestamp - error: now also carries the message of a transient (non-terminal) processing error. It holds the most recent message, transient or terminal; a terminal failure is identified by status == FAILED, not by error being non-null. recordError is best-effort — a failed write is logged and never changes control flow, so the command is still kept in the queue and retried. The streak is reset on recovery, with a single write and only when there is something to reset, so healthy re-polls stay write-free. Backward-compatible: the new fields default to 0/null when older serialized state is read. One deliberate adaptation from e54229e, required by this tree: 0.4.x still has executeWithTimeout, which wraps a handler exception in a generic RuntimeException("Command execution failed"). #89 was written against #84, which had removed that method, so recording e.getMessage() verbatim was correct there but here would stamp every transient error on the execute() path with the same useless string. recordError now records the root cause's message via rootMessage(), which is also correct for the checkStatus() path where the exception propagates directly. Caught by #89's own test asserting error == 'Persistent boom'; it failed with 'Command execution failed' before the adaptation. Tests: the two specs from #89 plus its CommandState serialization coverage. Module suite 18/18 green. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restores the fix originally released as 0.5.1 (6c7b171, #87) and dropped by the revert to 0.4.0 in #100. Same hunk, re-cut on top of the 0.4.0 tree. No VERSION or changelog change — code and tests only. The catch in CommandServiceImpl.processCommandWithHandler treated any escaping exception as a terminal command outcome: it persisted a FAILED CommandState and returned true, so RedisMessageStream.consume acked and deleted the entry. There was no retry. That conflates "the handler threw" with "the command failed", and it fails asymmetrically because the two live in different stores. Command state is in Redis; the domain work is in Postgres. When Micronaut closes the HikariCP pool while the queue is still draining, the handler throws a JDBC error and the catch records a permanent verdict using the store that still works, about a failure caused by the store that does not — while the domain entity was never transitioned. Queue empty, command FAILED, entity dangling, nothing left to advance it, polling clients hanging (seqeralabs/sched#712). Fix: log and return false. The entry stays unacked in the PEL for XAUTOCLAIM to hand to a live consumer, and since the catch never persists started(), status stays SUBMITTED so the next delivery re-enters execute(). A genuine failure is signalled by returning a FAILED CommandResult, which the terminal branch above already handles. Independent of the async/heartbeat-lease model that #100 removed: this works on the synchronous 1.5.0 stream because consume() only xacks/xdels when the consumer returns true. Unchanged: a returned FAILED CommandResult is still terminal, and an unknown command type is still failed and acked (no handler exists to retry) — both covered by existing tests. Test: the spec from #87, verified to fail against the 0.4.0 catch and pass with the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recovers the error tracking originally released as 0.6.0 (e54229e, #89), dropped by the revert to 0.4.0 in #100. No VERSION or changelog change — code and tests only. Companion to the retry fix in the parent commit: once a thrown handler is retried instead of terminal-failed, a command can retry indefinitely with nothing recording that it is happening. These fields make that visible. - errorsCount: consecutive processing errors since the last successful processing - modifiedAt: last-write timestamp - error: now also carries the message of a transient (non-terminal) processing error. It holds the most recent message, transient or terminal; a terminal failure is identified by status == FAILED, not by error being non-null. recordError is best-effort — a failed write is logged and never changes control flow, so the command is still kept in the queue and retried. The streak is reset on recovery, with a single write and only when there is something to reset, so healthy re-polls stay write-free. Backward-compatible: the new fields default to 0/null when older serialized state is read. One deliberate adaptation from e54229e, required by this tree: 0.4.x still has executeWithTimeout, which wraps a handler exception in a generic RuntimeException("Command execution failed"). #89 was written against #84, which had removed that method, so recording e.getMessage() verbatim was correct there but here would stamp every transient error on the execute() path with the same useless string. recordError now records the root cause's message via rootMessage(), which is also correct for the checkStatus() path where the exception propagates directly. Caught by #89's own test asserting error == 'Persistent boom'; it failed with 'Command execution failed' before the adaptation. Tests: the two specs from #89 plus its CommandState serialization coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cmd-queue 0.4.1) (#102) * fix(cmd-queue): retry a command whose handler throws Restores the fix originally released as 0.5.1 (6c7b171, #87) and dropped by the revert to 0.4.0 in #100. Same hunk, re-cut on top of the 0.4.0 tree. No VERSION or changelog change — code and tests only. The catch in CommandServiceImpl.processCommandWithHandler treated any escaping exception as a terminal command outcome: it persisted a FAILED CommandState and returned true, so RedisMessageStream.consume acked and deleted the entry. There was no retry. That conflates "the handler threw" with "the command failed", and it fails asymmetrically because the two live in different stores. Command state is in Redis; the domain work is in Postgres. When Micronaut closes the HikariCP pool while the queue is still draining, the handler throws a JDBC error and the catch records a permanent verdict using the store that still works, about a failure caused by the store that does not — while the domain entity was never transitioned. Queue empty, command FAILED, entity dangling, nothing left to advance it, polling clients hanging (seqeralabs/sched#712). Fix: log and return false. The entry stays unacked in the PEL for XAUTOCLAIM to hand to a live consumer, and since the catch never persists started(), status stays SUBMITTED so the next delivery re-enters execute(). A genuine failure is signalled by returning a FAILED CommandResult, which the terminal branch above already handles. Independent of the async/heartbeat-lease model that #100 removed: this works on the synchronous 1.5.0 stream because consume() only xacks/xdels when the consumer returns true. Unchanged: a returned FAILED CommandResult is still terminal, and an unknown command type is still failed and acked (no handler exists to retry) — both covered by existing tests. Test: the spec from #87, verified to fail against the 0.4.0 catch and pass with the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(cmd-queue): track command processing errors on CommandState Recovers the error tracking originally released as 0.6.0 (e54229e, #89), dropped by the revert to 0.4.0 in #100. No VERSION or changelog change — code and tests only. Companion to the retry fix in the parent commit: once a thrown handler is retried instead of terminal-failed, a command can retry indefinitely with nothing recording that it is happening. These fields make that visible. - errorsCount: consecutive processing errors since the last successful processing - modifiedAt: last-write timestamp - error: now also carries the message of a transient (non-terminal) processing error. It holds the most recent message, transient or terminal; a terminal failure is identified by status == FAILED, not by error being non-null. recordError is best-effort — a failed write is logged and never changes control flow, so the command is still kept in the queue and retried. The streak is reset on recovery, with a single write and only when there is something to reset, so healthy re-polls stay write-free. Backward-compatible: the new fields default to 0/null when older serialized state is read. One deliberate adaptation from e54229e, required by this tree: 0.4.x still has executeWithTimeout, which wraps a handler exception in a generic RuntimeException("Command execution failed"). #89 was written against #84, which had removed that method, so recording e.getMessage() verbatim was correct there but here would stamp every transient error on the execute() path with the same useless string. recordError now records the root cause's message via rootMessage(), which is also correct for the checkStatus() path where the exception propagates directly. Caught by #89's own test asserting error == 'Persistent boom'; it failed with 'Command execution failed' before the adaptation. Tests: the two specs from #89 plus its CommandState serialization coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#100 and #102 (which also carried #103) are now on master, so this branch retargets there instead of stacking on the revert branch. Resolution: - VERSION and changelog changes are dropped from this branch entirely. Master merged #102/#103 without bumping cmd-queue past 0.4.0 or adding a changelog entry, so versioning for the whole 0.4.x line is one decision to make in one place, not something this PR should pre-empt. - CommandServiceImpl and AbstractMessageStream keep this branch's side, which is master's tree plus the drain: the #102 catch and #103 recordError/rootMessage reached this branch by cherry-pick before they were squashed onto master, so both sides carry them and only the drain is genuinely new. Verified: the diff against master is now the drain and nothing else — 5 files, no VERSION, no changelog. Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Restores both module trees to their state at
8bad0f5([release] lib-data-stream-redis@1.5.0, 14 May 2026) — the commit where stream was 1.5.0 and cmd-queue 0.4.0, so a single revert point covers both.No
[release]marker: this is not meant to republish.publish.shskips versions already in the repo, so 1.5.0 / 0.4.0 would be no-ops anyway. Every source and test file in both modules is byte-identical to8bad0f5— onlychangelog.txtandbuild.gradlediffer.What is undone
bdf937411fdd3a,86ae9ac)6c7b171e54229eCommandState(errorsCount,error,modifiedAt) → 0.6.0 (plus1f124f0)f3f4ac4CommandStatusSUBMITTED→PENDING / RUNNING→PROCESSING → 0.7.0e8fe916d38afb0lib-cmd-queue-redistherefore depends onlib-data-stream-redisagain.Scope decisions
lib-data-workqueue/lib-data-workqueue-redisare left in place, untouched. They carry the lease-based design forward and were never published (the release list in.github/workflows/build.ymldoes not include them). They are now unwired from cmd-queue but self-contained — separate packages (io.seqera.data.workqueue.*vsio.seqera.data.stream.*), so no clash.Two build-infra bits from build(java): Java 25 for Micronaut libs, Java 17 elsewhere; drop virtual-thread default; fix api leak #94 are deliberately kept rather than reverted, since they are repo-wide conventions and not module API:
io.seqera.micronaut-library-conventionsplugin id (Java 25 target for Micronaut modules)Everything else in the two trees is byte-identical to
8bad0f5.History preservation
REVERTEDentry naming the withdrawn versions and their PRs, instead of silently rewinding.archive/workqueue-pre-revert(2ecc744), which also carries the unmerged invocation-lease rework.lib-data-stream-redis1.6.0" callout (a version this PR removes), and the migration spec is marked HISTORICAL.Fixes that must be re-landed (not carried by this PR)
This is a pure revert, so two fixes from the reverted range are dropped. Both are separable from the async/workqueue work that was rejected, and both belong in #101's 0.4.1 — sched's
fix/ordered-shutdown-drain(f8f09293f, sched#888) already pins0.4.1, so they arrive with no extra version bump:6c7b171) — a thrown handler is terminal again here (store.save(state.failed(...)); return true), which strands work when a transient error hits the handler (sched#712). Separable:RedisMessageStream.consumeonlyxack/xdels ontrue, soreturn falseparks the entry in the PEL forXAUTOCLAIM, and the catch never persistsstarted(), so status staysSUBMITTEDand the next delivery re-entersexecute(). Drain in-flight work before releasing the queue #101's 0.4.1 currently carries the 0.4.0 catch verbatim, so its drain narrows the window without closing it.CommandQueuepublicly extendsAbstractMessageStreamand exposesMessageConsumer, but the revert restoresimplementation project(':lib-data-stream-redis'). Same leak build(java): Java 25 for Micronaut libs, Java 17 elsewhere; drop virtual-thread default; fix api leak #94 fixed for the workqueue dep; breaks downstream subclassers in Gradle composite builds.Audited for anything else: #87 is the only fix-shaped entry across the whole reverted changelog range, #86's cmd-queue diff is pure rename/migration, and the 1.2.0
xautoclaimstarvation fix is intact in the reverted tree.Downgrade note (recorded in the cmd-queue changelog)
Not a rollout prerequisite. 0.7.0 was never deployed — sched pinned 0.5.1 → 0.6.0 → 0.4.0 (
a39502274, sched#772) and no other repo depends on this module. The downgrade that did happen, 0.6.0 → 0.4.0, is safe:JacksonEncodingStrategydisablesFAIL_ON_UNKNOWN_PROPERTIES, so 0.6.0'serrorsCount/error/modifiedAtare ignored on read.For the 0.7.0 case only: it persists state with the renamed
PENDING/PROCESSINGvalues and a 7-day TTL.PROCESSINGthrows instore.findById()at step 1 ofprocessCommand, outside the try/catch, so the entry is never acked — the listener survives but the entry re-claims and re-throws every claim-timeout, a poison entry in an ERROR loop rather than a silent skip. Self-heals at the TTL; flush the affected keys to downgrade sooner.Verification
./gradlew assemble— BUILD SUCCESSFUL (all 134 tasks):lib-data-stream-redis:test— 15 tests, 0 failures:lib-cmd-queue-redis:test— 15 tests, 0 failures🤖 Generated with Claude Code