Skip to content

[Data] Fix flaky test_streaming_split_stats#64290

Merged
goutamvenkat-anyscale merged 1 commit into
ray-project:masterfrom
goutamvenkat-anyscale:ak/fix-flaky-streaming-split-stats
Jun 23, 2026
Merged

[Data] Fix flaky test_streaming_split_stats#64290
goutamvenkat-anyscale merged 1 commit into
ray-project:masterfrom
goutamvenkat-anyscale:ak/fix-flaky-streaming-split-stats

Conversation

@goutamvenkat-anyscale

Copy link
Copy Markdown
Contributor

Why

test_streaming_split_stats is flaky. The task_output_backpressure_time* metrics on the running MapBatches operator are wall-clock timers for output backpressure, which is a timing race against the single, slower streaming_split(1) consumer. The value is genuinely nondeterministic across runs (sometimes 0, usually positive), but the test hardcoded it to zero (Z), so it only passed on the occasional fast run (~1/5 locally; also seen failing in CI).

Empirically, output backpressure is the normal state for this pipeline (nonzero ~every run), so the hardcoded Z was the less-representative outcome. There is no deterministic value to assert for a wall-clock timer, so the honest fix is to stop asserting its magnitude rather than force a value.

What

Scoped, test-only change inside test_streaming_split_stats:

  • Mark just the three timer values (average_task_output_backpressure_time_s, task_output_backpressure_time, task_output_backpressure_time_s) as not-asserted (a <varies> sentinel applied to both the expected and produced stats) — but only for the running MapBatches operator (first occurrence).
  • Everything else stays strictly checked:
    • task_submission_backpressure_time remains N.
    • The idle split operator's backpressure timers remain strictly asserted as 0.
    • All other fields and all other tests are untouched (no change to canonicalize() or gen_expected_metrics()).

This mirrors the existing precedent for obj_store_mem_used (collapsed to A because it's "zero or positive depending on the run").

Verification

  • test_streaming_split_stats: 10/10 pass (was 1/5).
  • test_dataset__repr__, test_dataset_stats_basic: still pass (strict, unchanged).
  • pre-commit: clean.

🤖 Generated with Claude Code

The task_output_backpressure_time* metrics on the running MapBatches
operator are wall-clock timers for output backpressure, which is a timing
race against the single, slower streaming-split consumer. The value is
genuinely nondeterministic across runs (sometimes 0, usually positive), but
the test hardcoded it to zero (Z), so it only passed on the occasional fast
run (~1/5 locally).

Mark just those three timer values as not-asserted (a "<varies>" sentinel on
both expected and produced) for the running operator only. Everything else
stays strictly checked: task_submission_backpressure_time remains N, and the
idle split operator's backpressure timers remain strictly asserted as 0.

Verified: test_streaming_split_stats now passes 10/10 (was 1/5);
test_dataset__repr__ and test_dataset_stats_basic still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@goutamvenkat-anyscale goutamvenkat-anyscale requested a review from a team as a code owner June 23, 2026 21:47
@goutamvenkat-anyscale goutamvenkat-anyscale added data Ray Data-related issues go add ONLY when ready to merge, run all tests labels Jun 23, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates test_streaming_split_stats in python/ray/data/tests/test_stats.py to handle nondeterministic task_output_backpressure_time metrics. It replaces these metrics with a <varies> sentinel in both the expected and produced stats to prevent flaky test failures caused by timing races. There are no review comments, so no feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@goutamvenkat-anyscale goutamvenkat-anyscale enabled auto-merge (squash) June 23, 2026 22:07
@goutamvenkat-anyscale goutamvenkat-anyscale merged commit c63e5bd into ray-project:master Jun 23, 2026
9 checks passed
@goutamvenkat-anyscale goutamvenkat-anyscale deleted the ak/fix-flaky-streaming-split-stats branch June 23, 2026 22:33
limarkdcunha pushed a commit to limarkdcunha/ray that referenced this pull request Jun 30, 2026
## Why

`test_streaming_split_stats` is flaky. The
`task_output_backpressure_time*` metrics on the running `MapBatches`
operator are wall-clock timers for output backpressure, which is a
timing race against the single, slower `streaming_split(1)` consumer.
The value is genuinely nondeterministic across runs (sometimes `0`,
usually positive), but the test hardcoded it to zero (`Z`), so it only
passed on the occasional fast run (~1/5 locally; also seen failing in
CI).

Empirically, output backpressure is the *normal* state for this pipeline
(nonzero ~every run), so the hardcoded `Z` was the less-representative
outcome. There is no deterministic value to assert for a wall-clock
timer, so the honest fix is to stop asserting its magnitude rather than
force a value.

## What

Scoped, test-only change inside `test_streaming_split_stats`:

- Mark just the three timer values
(`average_task_output_backpressure_time_s`,
`task_output_backpressure_time`, `task_output_backpressure_time_s`) as
**not-asserted** (a `<varies>` sentinel applied to both the expected and
produced stats) — but **only for the running `MapBatches` operator**
(first occurrence).
- Everything else stays strictly checked:
  - `task_submission_backpressure_time` remains `N`.
- The idle `split` operator's backpressure timers remain strictly
asserted as `0`.
- All other fields and all other tests are untouched (no change to
`canonicalize()` or `gen_expected_metrics()`).

This mirrors the existing precedent for `obj_store_mem_used` (collapsed
to `A` because it's "zero or positive depending on the run").

## Verification

- `test_streaming_split_stats`: **10/10 pass** (was 1/5).
- `test_dataset__repr__`, `test_dataset_stats_basic`: still pass
(strict, unchanged).
- `pre-commit`: clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data Ray Data-related issues go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants