Skip to content

CI: count-distinct byte tests run for hours and hit the 360-minute limit #25011

Description

@Phoenix500526

Describe the bug

The cargo test hash collisions (amd64) job can spend several hours in two count-distinct unit tests and reach the 360-minute execution limit. I noticed this while checking #24923, then found the same behavior on main and other branches.

The tests are in datafusion-functions-aggregate-common:

aggregate::count_distinct::bytes::tests::ungrouped_utf8_accumulator_is_never_worse_than_a_pre_allocated_set
aggregate::count_distinct::bytes::tests::ungrouped_utf8_view_accumulator_is_never_worse_than_a_pre_allocated_set

Examples from September 6-7, 2026:

All three jobs have the annotation: The job has exceeded the maximum execution time of 6h0m0s. These are timeout cancellations, not skipped tests.

It does not always time out: main at fdfb67d056 completed successfully. The Utf8 and Utf8View tests took approximately 2h39m and 4h31m respectively, and the crate reported 49 passed; 0 failed; ... finished in 16248.36s.

To Reproduce

The tests were added in #24857, merged as d25ffaab493c18690028410656ed12ea6c6fb0b6. On a checkout containing that commit, compare:

# Normal hashing
cargo test --profile ci -p datafusion-functions-aggregate-common \
  --lib ungrouped_utf8 -- --nocapture

# Forced collisions, as enabled in the affected CI job.
# This can run for hours; interrupt after collecting enough evidence.
cargo test --profile ci -p datafusion-functions-aggregate-common \
  --features datafusion-common/force_hash_collisions \
  --lib ungrouped_utf8 -- --nocapture

Expected behavior

The collision check should finish within the CI time limit while preserving the regression coverage these tests were designed to provide.

Is this already tracked, or is there a plan for handling these tests in the forced-collision job? In particular, what behavior or regression boundaries are the 100,000 and 500,000 cardinalities intended to protect, and what approach would preserve that coverage?

Additional context

The test cardinalities reach 500,000 distinct strings, inserted into both a lazy and a pre-allocated set. The comments describe checking allocation savings and capacity convergence. With force_hash_collisions, every hash is zero. Source inspection suggests quadratic collision checking explains the long runtime.

#24918 describes a related forced-collision problem, but fixes a different memory-limit test whose partitioning assumptions no longer hold. It does not address these two tests.

I would like to understand the intended coverage and the community's preferred approach before proposing any reduction in cardinality or skipping tests. This issue is reporting the CI impact, not proposing either change as the fix.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions