State store commits do not recover after a temporary failure, and redrive may produce unexpected behaviour

Open
  • Is something wrong?
#8,364 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
aws, java, sqlite
Domain
data-engineering

Research direction

Start in StateStoreCommitter.applyBatch and MultiThreadedStateStoreCommitter.applyBatchOfCommits, where failed requests are marked and the loop continues to later commits for the same table. Read StateStoreCommitterTest and StateStoreCommitterLambdaTest to see the current batch behaviour. Done means that a retryable failure on one table stops that table's later commits in the batch while other tables proceed, with the design decision on validation failures settled first.

Written by the indexing model from the issue text.

Description

bug needs-refinement under-discussion
Description / Background

The state store committer reads from a FIFO queue grouped by table ID, so that each table's commits are applied in order. When a commit request fails, StateStoreCommitter.applyBatch marks only that request as failed and goes on to apply the later requests for the same table.

There's a separate issue where with the lambda committer, failed commits are deleted and lost instead of going to the dead letter queue, see #8618.

When messages do fail, and are correctly reported as failed to SQS, currently they are immediately sent to the dead letter queue, because the queue is configured to only allow a single attempt. If we configured retries on the queue, or if you redrive messages from the dead letter queue, they will be attempted after later commits for that table, and the transaction log will end up out of order.

AWS's guidance for FIFO queues with partial batch responses is to stop at the first failure in a message group and report that message and every later message in the same group as failed.

Steps to reproduce
  1. Deploy an instance using the transaction log state store.
  2. Send three commit requests for the same table to the state store committer queue so they land in one batch: A, B and C, where B fails in a retryable way. Currently if a commit points to a large transaction body key that doesn't exist, that is treated as retryable, so you could set up B with a missing transaction body.
  3. A and C are applied. B goes to the dead letter queue, and if it later succeeds due to a redrive, it is applied after C.
Expected behaviour
  • A retryable failure on a commit for table T stops processing of T's later commits in that batch. Those are reported as failed and retried in their original order.
  • Commits for other tables in the same batch carry on as normal.
  • A commit that keeps failing ends up on the committer DLQ after maxReceiveCount attempts, and the alarm fires.
  • Validation failures that will never succeed are handled as the team agrees (see below), without blocking the table's queue indefinitely.
Technical Notes / Implementation Details

Design decision needed first: which failures should be retried?

Most RuntimeExceptions that reach the loop are validation failures from a transaction's validate, which will fail on every retry. Examples:

  • AddFilesTransaction: file already exists
  • AssignJobIdsTransaction: reference already assigned to a job
  • SplitPartitionTransaction / ExtendPartitionTreeTransaction: partition missing or already split
  • DeleteFilesTransaction: file still has references

If these are treated as retryable under correct FIFO behaviour, they block that table's whole commit queue until they use up maxReceiveCount. During that time no compactions, ingests or splits commit for the table. A suggested split:

  • Validation StateStoreExceptions: log them, report them to the relevant tracker, and delete the message.
  • Infrastructure or transient errors (S3, DynamoDB beyond throttling retries): retry, and fail the rest of that table's group.

ReplaceFileReferencesTransaction (compaction commits) mostly avoids this already. Its validate does nothing, and apply skips invalid jobs rather than throwing.

Code changes:

  • java/statestore-committer-core/../StateStoreCommitter.java, in applyBatch: on a retryable failure for table T, mark that request and every later request for T as failed, and keep processing other tables. The existing InterruptedException branch already marks the rest as failed, but for every table rather than per table.
  • java/statestore-committer/../MultiThreadedStateStoreCommitter.java, in applyBatchOfCommits: the same change. Also consider the existing TODOs there: the fixed 250ms retry sleep, and commits possibly taking longer than the 15-minute visibility timeout.
  • java/../stack/core/StateStoreCommitterStack.java: add .reportBatchItemFailures(true) to the commit queue event source. Make this change in the same PR as the logic fix or after it, never before.

Testing:

  • Unit tests in StateStoreCommitterTest / StateStoreCommitterLambdaTest covering:
    • A retryable failure on commit B means B and C are reported, and A isn't.
    • Another table in the same batch is unaffected.
    • Validation failures are handled as the team decided.
  • Equivalent tests for MultiThreadedStateStoreCommitter.
  • A system test in StateStoreCommitterST (and ECSStateStoreCommitterST if the EC2 path is in scope), using the A/B/C scenario above. Assert that log order is preserved, the second table is unaffected, and B ends up on the DLQ. The system test instance will need low maxReceiveCount and visibility timeouts.
  • Update the approved CDK template (default-instance.approved.json) for the committer mapping. If the focused CDK assertion from #8363 excludes the committer, remove that exclusion.

Related: #8363

Screenshots/Logs

StateStoreCommitter logs this for each failed request before carrying on with the rest of the batch:

Failed commit request
Dominant language
Java
Stars
107
Forks
29
Avg merge
1d 33m
Merged PRs (30d)
115

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from gchq/sleeper

All issues in gchq/sleeper

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.