Skip to content

GH-51011: [C++][CI] Poll for GCS testbench readiness instead of one 10s attempt - #51012

Open
jonasdedden wants to merge 1 commit into
apache:mainfrom
jonasdedden:gcs-testbench-readiness
Open

jonasdedden wants to merge 1 commit into
apache:mainfrom
jonasdedden:gcs-testbench-readiness

Conversation

@jonasdedden

@jonasdedden jonasdedden commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

arrow-gcsfs-test sometimes fails on CI with Could not start GCS emulator 'storage-testbench' (failed to listen). The testbench does listen, it is just slow to answer: it runs under Werkzeug's reloader, which binds the socket in the parent and serves from a child that re-imports grpcio, protobuf and flask first.

GcsTestbench passes its 10s startup budget to LimitedTimeRetryPolicy as well, so the readiness loop only makes one attempt. And since stderr is discarded, the CI log can't tell a slow start from a crash.

What changes are included in this PR?

In gcsfs_test.cc:

  • 5s per attempt, 60s overall, so the loop actually polls
  • keep the testbench's stderr
  • error message says (did not become ready) instead of (failed to listen)

util::Process::IgnoreStderr is now unused; I can remove it if preferred.

Are these changes tested?

By arrow-gcsfs-test itself. I couldn't reproduce the CI flake on demand, so this widens the margin and makes the next failure diagnosable.

Are there any user-facing changes?

No.

@jonasdedden
jonasdedden requested a review from pitrou as a code owner August 26, 2026 11:13
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #51011 has been automatically assigned in GitHub to PR creator.

@jonasdedden

Copy link
Copy Markdown
Contributor Author

CI failure "Invalid: Attempt to initialize S3 after it has been finalized." because of #50930

@uros-b

uros-b commented Aug 29, 2026

Copy link
Copy Markdown
Member

Comments seem a bit verbose, but otherwise LGTM!

… one 10s attempt

The 10s startup budget was also passed to LimitedTimeRetryPolicy, so the
readiness loop made a single attempt. Use 5s per attempt and 60s overall,
and keep the testbench's stderr so startup failures show up in CI logs.
@jonasdedden
jonasdedden force-pushed the gcs-testbench-readiness branch from e332217 to 39a1980 Compare September 25, 2026 19:25
@jonasdedden

Copy link
Copy Markdown
Contributor Author

@uros-b thinned it down a bit

@uros-b

uros-b commented Sep 26, 2026

Copy link
Copy Markdown
Member

Nice, thank you @jonasdedden! Could we get green CI here? Also, please ping expert committers for further review

@jonasdedden

Copy link
Copy Markdown
Contributor Author

Thanks @uros-b! The two remaining failures are unrelated flakes that also occur on main:

Meanwhile, the GCS "failed to listen" flake this PR addresses hit main again on 2026-09-26 (run 36235750984 (https://github.com/apache/arrow/actions/runs/36235750984)), whilst not appearing anywhere in this PR AFAIS.

@pitrou could you take a look? (cc @kou for the CI side)

@kou kou 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.

+1

@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants