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 — [connectors] Make the postgres runtime shutdown panic test robust on CI by blp · Pull Request #7808 · feldera/feldera

Conversation

@blp

@blp blp commented Nov 3, 2026

Copy link
Copy Markdown
Member

Fixes #7716.

a_panic_on_the_worker_task_still_shuts_the_runtime_down_in_time failed intermittently in the merge queue on PRs unrelated to connectors.

Cause

The test gave the wrapper thread 6s (RUNTIME_SHUTDOWN_TIMEOUT + 5s). The deliberate panic! runs the panic hook before the unwind starts, and the Adapter Tests job sets RUST_BACKTRACE=1, so the hook resolves a full backtrace from the large debug test binary. Under CI load that, plus shutdown_timeout, could take more than 6s although the runtime shut down in time.

Changes (test only)

Change Effect
Raise the panic with resume_unwind in place of panic! Skips the panic hook, so no backtrace is resolved
The stuck blocking task waits on a channel the test releases, not sleep(30s) A regression hangs without end, so the timeout can go from 6s to 30s without a false pass
Panic only after the blocking task reports that it runs Before, yield_now did not ensure this. A queued blocking task is dropped at shutdown, so the test could pass without testing the hang

Testing

Run Result
Fixed test, RUST_BACKTRACE=1 Passes in 1.00s
Fixed test with shutdown_timeout in block_on_then_shut_down replaced by drop(runtime) Fails at the 30s timeout
cargo fmt --check Clean

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@blp
[connectors] Make the postgres runtime shutdown panic test robust on CI.
7bda391 
`a_panic_on_the_worker_task_still_shuts_the_runtime_down_in_time` gave
the wrapper thread only 6s, and that time also paid for the panic hook,
which resolves a full backtrace under `RUST_BACKTRACE=1`. Under CI load
that took longer than 6s, so the test failed although the runtime shut
down in time.

- Raise the deliberate panic with `resume_unwind`, which skips the hook.
- Keep the blocking task stuck until the test releases it, so that a
  regression hangs without end, and raise the timeout to 30s.
- Panic only after the blocking task runs. Before, `yield_now` did not
  ensure this, and a queued task is dropped at shutdown, so the test
  could pass without testing the hang.

Fixes #7716.

Signed-off-by: Ben Pfaff <[email protected]>
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

LGTM.

Check Result
cargo test -p dbsp_connectors --lib a_panic_on_the_worker_task with RUST_BACKTRACE=1 pass, 1.00s
30 repeated runs 30/30 pass, 1.00–1.01s
Mutation: shutdown_timeout → drop(runtime) in block_on_then_shut_down fails as expected (40.7s)
  • The mutation run needed about 10s more than the 30s timeout, which is the backtrace for the failing expect. That supports the PR's diagnosis that the panic hook made the old 6s budget flaky.
  • Flaky-test gate: passes. The test waits on a started handshake instead of yield_now, and the blocking task stays stuck until the test releases it, so the 30s timeout cannot hide a hang. If the test fails, release_tx is dropped during the unwind, so the stuck thread does not leak.
  • This is a test-only change, so the docs, breaking-change and SQL-type gates do not apply. The commit message is good.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bestouff
bestouff approved these changes Nov 3, 2026
View reviewed changes
@blp
blp added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit d1f666a Nov 4, 2026
3 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@blp
blp deleted the fix-flaky-runtime-shutdown-test branch November 4, 2026 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bestouff bestouff bestouff approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flaky: a_panic_on_the_worker_task_still_shuts_the_runtime_down_in_time hangs past its shutdown-timeout margin

Uh oh!

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

2 participants

@blp @bestouff
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.