GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [dbsp] Keep the circuit running after a failed checkpoint by blp · Pull Request #7846 · feldera/feldera

Conversation

@blp

@blp blp commented Nov 4, 2026

Copy link
Copy Markdown
Member

A failed checkpoint no longer kills the circuit. PrepareCheckpoint only takes in-memory snapshots of operator state, so the circuit stays consistent after it fails. The controller already treats a checkpoint error as non-fatal and retries later.

Closes #7844.

Describe Manual Test Plan

The new test test_failed_checkpoint_keeps_circuit fails both with the old kill-on-error (Runtime(Terminated)) and with an early return that does not drain the other workers' replies (Expected CommitProgress response, got CheckpointPrepared([])).

Checklist

  • Unit tests added/updated
  • Integration tests added/updated
  • Documentation updated
  • Changelog updated

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@blp blp added bug Something isn't working DBSP core Related to the core DBSP library labels Nov 4, 2026
@blp blp self-assigned this Nov 4, 2026
mihaibudiu
mihaibudiu approved these changes Nov 4, 2026
View reviewed changes
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

LGTM.

What I ran

Check Result
cargo test --locked -p dbsp --lib test_failed_checkpoint_keeps_circuit, 20 runs pass every time, no flakiness
cargo test --locked -p dbsp --lib circuit::dbsp_handle 32 pass, 1 fail: test_panic_in_tokio_merger_runtime. It also fails with the PR reverted, so the PR did not cause it (it times out in this sandbox).

Checked the core claim. PrepareCheckpoint is side-effect free. CircuitHandle::checkpoint only calls Node::checkpoint. I read the z1, transaction_z1, trace, accumulate_trace, window, output and spine save implementations. Each one only snapshots or serializes in memory, and none does an exchange. A failure part-way through therefore leaves the workers consistent. Draining the remaining replies before returning is the right fix for the stale-reply problem.

Test gaps (medium):

  • The new non-fatal Ok(Err(e)) => return Err(e) arm in unicast_command has no test. Nothing unicasts PrepareCheckpoint today, so this arm is unreachable for now.
  • No test covers every worker failing at once. The current test fails only worker 0.
  • No controller-level test checks that a failed periodic checkpoint keeps the pipeline running and that a later checkpoint succeeds. This is the behaviour A worker error on a DBSP command, such as a failed checkpoint, kills the circuit #7844 reports.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@blp
blp added this pull request to the merge queue Nov 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2026

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu
mihaibudiu added this pull request to the merge queue Nov 5, 2026
@mythical-fred-oss mythical-fred-oss Bot mentioned this pull request Nov 5, 2026
arm64 Runtime Integration Tests: OIDC token starts getting 401 'credential rejected' ~40min into the run #7048
Closed
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Verdict: infrastructure, not this PR. High confidence.

  • Failing job: Integration Tests / Runtime Integration Tests (arm64)
  • The only failure is tests/runtime/test_udf.py::TestUDF::test_legacy_variant_udf, which gets 401 credential rejected (invalid or expired token / API key) on its very first API call (log lines 455, 607-608, 654, 748-749) — the shared CI OIDC token going stale mid-run, not a test or product bug.
  • This PR's diff is entirely crates/dbsp/src/circuit/dbsp_handle.rs (checkpoint/circuit resume logic) and has nothing to do with the Python client, UDFs, or CI auth.
  • This is a known, recurring issue: tracked in #7048, where it's hit several unrelated PRs over the past weeks. I've added this run's evidence there as another occurrence.

Next step: re-queueing should be fine — the same shared instance instability could recur, but there's nothing to fix in this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2026

Uh oh!

There was an error while loading. Please reload this page.

@blp
blp added this pull request to the merge queue Nov 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2026

Uh oh!

There was an error while loading. Please reload this page.

@blp
[dbsp] Keep the circuit running after a failed checkpoint.
3c6ce0a
A worker error reply to any command made `DBSPHandle` kill the runtime,
so a failed checkpoint stopped the pipeline.  `PrepareCheckpoint` only
takes in-memory snapshots of operator state, so a failure leaves the
circuit consistent.  Each command now says whether an error from it is
fatal, and only `PrepareCheckpoint` is not.

After a non-fatal error, `broadcast_command` still reads the replies of
the other workers, so that no stale reply is left for the next command.

The regression test for #7075 used a failed checkpoint to make the
circuit thread kill the runtime.  A failed checkpoint no longer does
that, so this adds a test-only command, `InjectFatalError`, behind the
new `testing` feature of `dbsp`, that every worker fails fatally.
`Controller::inject_fatal_error` sends it from the circuit thread.  The
test, now `worker_error_does_not_deadlock`, checks that a failed
checkpoint leaves the circuit running and that a fatal error does not
deadlock.

Fixes #7844.

Signed-off-by: Ben Pfaff <[email protected]>
@blp
blp force-pushed the checkpoint-error-keeps-circuit branch from c0e5749 to 3c6ce0a Compare November 5, 2026 21:05
@blp
blp enabled auto-merge November 5, 2026 21:05
@blp
blp disabled auto-merge November 5, 2026 21:45
@blp
blp added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 855cd31 Nov 6, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@blp
blp deleted the checkpoint-error-keeps-circuit branch November 6, 2026 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@blp blp

Labels

bug Something isn't working DBSP core Related to the core DBSP library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

A worker error on a DBSP command, such as a failed checkpoint, kills the circuit

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@blp @mihaibudiu
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.