Skip to content

minor: speedup one SPM unit test - #26929

Merged
alamb merged 2 commits into
apache:mainfrom
2010YOUY01:test-speed/round-robin-merge
Oct 30, 2026
Merged

alamb merged 2 commits into
apache:mainfrom
2010YOUY01:test-speed/round-robin-merge

Conversation

@2010YOUY01

Copy link
Copy Markdown
Member

Important

Fork CI setup: For pull requests from a fork to main, enable workflows in your fork's Actions tab before you push.
See the fork CI setup guide.

Which issue does this PR close?

  • Closes #.

Rationale for this change

Make one UT faster while preserving the test goal. The goal is preserved since no assertion requires update.

cargo nextest run -p datafusion-physical-plan --lib --test-threads 1 test_round_robin_tie_breaker_

Success case:  7.093s → 1.846s
Failure case:  0.520s → 0.532s

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user-facing changes?

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Oct 30, 2026
@alamb
alamb added this pull request to the merge queue Oct 30, 2026
@alamb

alamb commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

🚀

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@alamb
alamb added this pull request to the merge queue Oct 30, 2026
@alamb

alamb commented Oct 30, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-10-30 at 4 02 50 PM BTW I looked into why this was booted from the queue event hough all the tests pass -- it seems it is because it ran on a RustOn runner that was a spot instance and got reclaimed. It passed on retry, but at that point it had already been removed from the queue 🤔

Merged via the queue into apache:main with commit 611311c Oct 30, 2026
44 checks passed
Phoenix500526 pushed a commit to Phoenix500526/datafusion that referenced this pull request Nov 3, 2026
…sh_collisions (apache#27034)

## Which issue does this PR close?

- Closes apache#27033.

## Rationale for this change

`completed_groups_release_first_value_bitmap` (added in apache#26782) takes
about 10 minutes in the `cargo test hash collisions (amd64)` merge-queue
job. It aggregates 131,072 distinct keys in one batch. Under
`force_hash_collisions`, every key has the hash 0, so the aggregate is
quadratic. The test checks that `FIRST_VALUE(v ORDER BY v)` releases its
per-batch bitmap, which does not depend on hashing.

The long job gives spot runner shutdowns more time to hit it. Evidence
from merge-queue runs:

- In the passing rerun for apache#26926
([job](https://github.com/apache/datafusion/actions/runs/41571721394/job/125710730373)),
the physical-plan unit test binary took 609.89s. All other tests in it
finished by 03:40:31. This test ran from about 03:40:07 to 03:50:17.
- The median job time went from 6.2 minutes (230 jobs, Oct 21 to Oct 30)
to 12.0 minutes (70 jobs after apache#26782 merged).
- All 5 failed attempts of the job since then were runner shutdowns
while this test was running: apache#26929
([job](https://github.com/apache/datafusion/actions/runs/41114163010/job/124263887540)),
apache#26898
([job](https://github.com/apache/datafusion/actions/runs/41115099549/job/124267135386)),
apache#26883
([job](https://github.com/apache/datafusion/actions/runs/41120437208/job/124285496814)),
apache#26974
([job](https://github.com/apache/datafusion/actions/runs/41559552415/job/125666576696)),
apache#26926
([job](https://github.com/apache/datafusion/actions/runs/41571721394/job/125705896210)).

## What changes are included in this PR?

Skip the test under `force_hash_collisions`, as apache#26594 did for another
test. The imports that only this test uses move into its body, so they
are not unused under the feature.

## What is the testing strategy for this PR?

Test config change only. The regular `cargo test` job still runs the
test. Locally (`--profile ci`), the test takes 0.92s by default and 309s
with `force_hash_collisions`. With the feature, it is now skipped, and
Clippy passes with and without the feature.

## Are there any user-facing changes?

No.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants