Skip to content

fix(orca): make orca's redis queue handling more robust - #8002

Open
apoorvmahajandd wants to merge 4 commits into
spinnaker:mainfrom
apoorvmahajandd:fix/orca-queue-handling
Open

fix(orca): make orca's redis queue handling more robust#8002
apoorvmahajandd wants to merge 4 commits into
spinnaker:mainfrom
apoorvmahajandd:fix/orca-queue-handling

Conversation

@apoorvmahajandd

@apoorvmahajandd apoorvmahajandd commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Motivation

We saw multiple stuck/zombie executions with these stacktraces:

redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out

java.lang.IllegalArgumentException: Unable to map context to class com.netflix.spinnaker.orca.clouddriver.tasks.manifest.WaitForManifestStableContext. Error: Cannot construct instance of `java.util.ArrayList` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out

Suppressed: redis.clients.jedis.exceptions.JedisConnectionException: Attempting to read from a broken connection

Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `java.util.ArrayList` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('java.lang.IllegalArgumentException: Unable to map context to class com.netflix.spinnaker.orca.clouddriver.tasks.manifest.WaitForManifestStableContext. Error: Cannot construct instance of `java.util.ArrayList` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out

Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `java.util.ArrayList` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out

java.lang.IllegalArgumentException: Unable to map context to class com.netflix.spinnaker.orca.clouddriver.tasks.manifest.WaitForManifestStableContext. Error: Cannot construct instance of `java.util.ArrayList` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out

The stacktrace showed:

"exception": {
"details": {
"error": "Unexpected Task Failure",
"errors": [
"java.net.SocketTimeoutException: Read timed out"
],
"stackTrace": "redis.clients.jedis.exceptions.JedisConnectionException: java.net.SocketTimeoutException: Read timed out\n\tat redis.clients.jedis.util.RedisInputStream.ensureFill(RedisInputStream.java:205)\n\tat redis.clients.jedis.util.RedisInputStream.readByte(RedisInputStream.java:43)\n\tat redis.clients.jedis.Protocol.process(Protocol.java:165)\n\tat redis.clients.jedis.Protocol.read(Protocol.java:230)\n\tat redis.clients.jedis.Connection.readProtocolWithCheckingBroken(Connection.java:352)\n\tat redis.clients.jedis.Connection.getMany(Connection.java:364)\n\tat redis.clients.jedis.Transaction.exec(Transaction.java:42)\n\tat com.netflix.spinnaker.q.redis.AbstractRedisQueue.multi$keiko_redis(AbstractRedisQueue.kt:76)\n\tat com.netflix.spinnaker.q.redis.RedisQueue.queueMessage$keiko_redis(RedisQueue.kt:304)\n\tat com.netflix.spinnaker.q.redis.RedisQueue.push(RedisQueue.kt:144)\n\tat com.netflix.spinnaker.q.Queue$DefaultImpls.push(Queue.kt:57)\n\tat com.netflix.spinnaker.q.metrics.MonitorableQueue$DefaultImpls.push(MonitorableQueue.kt:26)\n\tat com.netflix.spinnaker.q.redis.AbstractRedisQueue.push(AbstractRedisQueue.kt:23)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler$handle$1$1.invoke(StartStageHandler.kt:122)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler$handle$1$1.invoke(StartStageHandler.kt:95)\n\tat com.netflix.spinnaker.orca.q.handler.AuthenticationAware$DefaultImpls.withAuth$lambda-0(AuthenticationAware.kt:51)\n\tat com.netflix.spinnaker.security.AuthenticatedRequest.lambda$wrapCallableForPrincipal$0(AuthenticatedRequest.java:272)\n\tat com.netflix.spinnaker.orca.q.handler.AuthenticationAware$DefaultImpls.withAuth(AuthenticationAware.kt:51)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler.withAuth(StartStageHandler.kt:61)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler$handle$1.invoke(StartStageHandler.kt:95)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler$handle$1.invoke(StartStageHandler.kt:80)\n\tat com.netflix.spinnaker.orca.q.handler.OrcaMessageHandler$withStage$1.invoke(OrcaMessageHandler.kt:86)\n\tat com.netflix.spinnaker.orca.q.handler.OrcaMessageHandler$withStage$1.invoke(OrcaMessageHandler.kt:75)\n\tat com.netflix.spinnaker.orca.q.handler.OrcaMessageHandler$DefaultImpls.withExecution(OrcaMessageHandler.kt:96)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler.withExecution(StartStageHandler.kt:61)\n\tat com.netflix.spinnaker.orca.q.handler.OrcaMessageHandler$DefaultImpls.withStage(OrcaMessageHandler.kt:75)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler.withStage(StartStageHandler.kt:61)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler.handle(StartStageHandler.kt:80)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler.handle(StartStageHandler.kt:61)\n\tat com.netflix.spinnaker.q.MessageHandler$DefaultImpls.invoke(MessageHandler.kt:36)\n\tat com.netflix.spinnaker.orca.q.handler.OrcaMessageHandler$DefaultImpls.invoke(OrcaMessageHandler.kt:46)\n\tat com.netflix.spinnaker.orca.q.handler.StartStageHandler.invoke(StartStageHandler.kt:61)\n\tat com.netflix.spinnaker.orca.q.audit.ExecutionTrackingMessageHandlerPostProcessor$ExecutionTrackingMessageHandlerProxy.invoke(ExecutionTrackingMessageHandlerPostProcessor.kt:72)\n\tat com.netflix.spinnaker.q.QueueProcessor$callback$1.invoke$lambda-0(QueueProcessor.kt:90)\n\tat java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)\n\tat java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)\n\tat java.base/java.lang.Thread.run(Thread.java:840)\n\tSuppressed: redis.clients.jedis.exceptions.JedisConnectionException: Attempting to read from a broken connection\n\t\tat redis.clients.jedis.Connection.readProtocolWithCheckingBroken(Connection.java:348)\n\t\tat redis.clients.jedis.Connection.getMany(Connection.java:364)\n\t\tat redis.clients.jedis.Transaction.discard(Transaction.java:83)\n\t\tat redis.clients.jedis.Transaction.clear(Transaction.java:36)\n\t\tat redis.clients.jedis.Transaction.close(Transaction.java:96)\n\t\tat kotlin.io.CloseableKt.closeFinally(Closeable.kt:59)\n\t\tat com.netflix.spinnaker.q.redis.AbstractRedisQueue.multi$keiko_redis(AbstractRedisQueue.kt:74)\n\t\t... 29 more\nCaused by: java.net.SocketTimeoutException: Read timed out\n\tat java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:288)\n\tat java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:314)\n\tat java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:355)\n\tat java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:808)\n\tat java.base/java.net.Socket$SocketInputStream.read(Socket.java:966)\n\tat java.base/java.io.InputStream.read(InputStream.java:218)\n\tat redis.clients.jedis.util.RedisInputStream.ensureFill(RedisInputStream.java:199)\n\t... 36 more\n"
},
"exceptionType": "JedisConnectionException",
"operation": "Check precondition (expression)",
"shouldRetry": false,
"timestamp": 1775665189165
}
  • This leads to us to believe that redis connectivity issues are another contributing factor in some pipeline executions becoming stuck or zombies.

Changes

Layer 1: Redis Queue Retry with Exponential Backoff

Problem: RedisQueue.push() had no retry logic. A single transient JedisConnectionException during queue.push() would immediately propagate up, potentially leaving pipelines stuck.

New files:

  • keiko-redis/RedisRetryConfig.kt — Config data class with maxAttempts=3, backoffMs=100, exponentialBackoff=true.

Modified files:

  • keiko-redis/AbstractRedisQueue.kt — Added retryConfig constructor param and a retry() method that wraps a block with up to maxAttempts retries, exponential backoff, and catches JedisException.
  • keiko-redis/RedisQueue.kt — Passes retryConfig to super, wraps push() body in retry { }.
  • keiko-redis/RedisClusterQueue.kt — Same changes as RedisQueue.
  • keiko-redis-spring/RedisQueueProperties.kt — Added var retry: RedisRetryConfig property, configurable via keiko.queue.redis.retry.*.
  • keiko-redis-spring/RedisQueueConfiguration.kt — Passes redisQueueProperties.retry to both RedisQueue and RedisClusterQueue constructors.

Layer 2: JedisExceptionHandler Safety Net

Problem: When all queue-level retries are exhausted, the JedisConnectionException reaches the handler's catch blocks. Without an ExceptionHandler that recognizes Jedis exceptions as retryable, StartStageHandler would permanently fail the stage.

New files:

  • orca-redis/JedisExceptionHandler.java — Implements ExceptionHandler. Walks the cause chain using instanceof JedisException (proper type check since it's in orca-redis which has Jedis on classpath). Returns
    shouldRetry=true.

Modified files:

  • orca-redis/RedisConfiguration.java — Registers JedisExceptionHandler as a Spring bean.

Layer 3: Handler Status Reset + Rethrow

Problem: When a handler persists a status change (e.g., RUNNING → SUCCEEDED) then fails on a subsequent queue.push(), keiko's ack-timeout retries the message. But the handler's status guard (e.g., stage.status in setOf(RUNNING, NOT_STARTED)) blocks the retried message because the status was already changed. Pipeline is permanently stuck.

Fix pattern: Save originalStatus before mutating, wrap queue operations in try/catch, restore exact original status on failure, persist via SQL, rethrow so keiko's ack-timeout retries the message.

StartStageHandler.kt

  • Inner catch (retryable path): Added stage.status = NOT_STARTED + repository.storeStage(stage) before queue.push(message, retryDelay) so the retried message passes the status != NOT_STARTED guard.
  • Outer catch: Changed from unconditionally marking failure to checking exceptionDetails?.shouldRetry. If retryable: resets status to NOT_STARTED, persists, rethrows (so QueueProcessor doesn't ack). If not
    retryable: existing path (mark beforeStagePlanningFailed, push CompleteStage).
  • Added comments explaining the status guard and the ordering of stage.status = RUNNING before stage.start().

SkipStageHandler.kt

  • Saves originalStatus before setting SKIPPED.
  • Wraps stage.startNext() in try/catch. On failure: restores originalStatus, persists, rethrows.

CompleteStageHandler.kt

  • Saves originalStatus at top of the if (stage.status in setOf(RUNNING, NOT_STARTED)) block.
  • Wraps the entire queue operations block (startNext(), CancelStage, CompleteExecution, parent propagation) in try/catch. On failure: restores originalStatus, persists, rethrows.

AbortStageHandler.kt

  • Saves originalStatus before setting TERMINAL.
  • Wraps queue operations (CancelStage, CompleteExecution/CompleteStage) in try/catch. On failure: restores originalStatus, persists, rethrows.

StartExecutionHandler.kt

  • Wraps initialStages.forEach { queue.push(StartStage(it)) } in try/catch. On failure: restores execution to NOT_STARTED, persists, rethrows. (Duplicate StartStage messages from partial pushes are safe —
    StartStageHandler's guard drops them once the stage moves to RUNNING.)

Tests

Each handler's existing Spek test file got a new describe("handling transient Redis exceptions") block:

  • StartStageHandlerTest.kt (+110 lines) — Two scenarios: (1) inner catch resets to NOT_STARTED and retries via queue push with delay; (2) outer catch resets to NOT_STARTED and rethrows JedisConnectionException.
  • SkipStageHandlerTest.kt (+43 lines) — startNext() throws → status restored to RUNNING, exception rethrown.
  • CompleteStageHandlerTest.kt (+43 lines) — startNext() throws via CompleteExecution push → status restored to RUNNING, exception rethrown.
  • AbortStageHandlerTest.kt (+42 lines) — CancelStage push throws → status restored to RUNNING, exception rethrown.
  • StartExecutionHandlerTest.kt (+38 lines) — StartStage push throws → execution status restored to NOT_STARTED, exception rethrown.

* add retry for redis

* fix

* add exponential backoff

* Refactor

* refactor

* handle silent drops

* add comment explaining why

* add same logic in other handlers
@apoorvmahajandd
apoorvmahajandd marked this pull request as ready for review September 12, 2026 01:21
abstract fun cacheScript()
abstract var readMessageWithLockScriptSha: String

protected fun <T> retry(block: () -> T): T {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ONLY wondering if we shouldn't be using resillience4j kinda thing for this...this would work, just wondering about using standard libraries

@jasonmcintosh jasonmcintosh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good with this MINUS a "should we be using resillience4j" - which could be a different PR to refactor retries instead and we move with this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants