Skip to content

[tests] replace fixed tokio::time::sleep workarounds with wait_for_table_ready helper.#564

Merged
fresh-borzoni merged 3 commits into
apache:mainfrom
slfan1989:fluss-562
May 28, 2026
Merged

[tests] replace fixed tokio::time::sleep workarounds with wait_for_table_ready helper.#564
fresh-borzoni merged 3 commits into
apache:mainfrom
slfan1989:fluss-562

Conversation

@slfan1989
Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #562

This PR replaces fixed tokio::time::sleep workarounds in Rust integration tests with polling-based readiness helpers. This makes the tests less brittle on slow CI and avoids unnecessary waiting when the table or partition bucket leaders are ready earlier.

Brief change log

  • Added readiness helpers in Rust integration test utils:
    • wait_for_table_ready
    • wait_for_table_buckets_ready
    • wait_for_partitions_ready
    • wait_for_partition_ready
    • wait_for_partition_buckets_ready
  • Replaced fixed sleeps after table creation with table readiness polling.
  • Replaced fixed sleeps after partition creation with partition readiness polling.
  • Updated the RecordBatchLogReader integration tests to use the new helpers, including the multi-bucket case.

Tests

Exists Test.

API and Format

No.

Documentation

No.

Comment thread crates/fluss/tests/integration/utils.rs
Copy link
Copy Markdown
Contributor

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

tiny comment

Comment thread crates/fluss/tests/integration/log_table.rs
Copy link
Copy Markdown
Member

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

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

@slfan1989 Ty for the PR, LGTM overall, minor comment

nit: these three wait_for_* loops (+ wait_for_cluster_ready_with_sasl) are the same poll-until-timeout skeleton copy-pasted, differing only in the probe. Could pull out a small poll_until(timeout, interval, || probe) helper - basically the Rust version of Java's CommonTestUtils.waitUtil/waitValue.
Not blocking, just a thought since we're here.

@slfan1989
Copy link
Copy Markdown
Contributor Author

@slfan1989 Ty for the PR, LGTM overall, minor comment

nit: these three wait_for_* loops (+ wait_for_cluster_ready_with_sasl) are the same poll-until-timeout skeleton copy-pasted, differing only in the probe. Could pull out a small poll_until(timeout, interval, || probe) helper - basically the Rust version of Java's CommonTestUtils.waitUtil/waitValue. Not blocking, just a thought since we're here.

Thanks for the suggestion! Updated. I factored out the shared poll-until-timeout skeleton into a small poll_until helper and reused it in the table/partition readiness helpers as well as wait_for_cluster_ready_with_sasl.

Copy link
Copy Markdown
Member

@fresh-borzoni fresh-borzoni left a comment

Choose a reason for hiding this comment

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

@slfan1989 Thank you for the contribution, LGTM 👍
@charlesdong1991 all good from your side?

@charlesdong1991
Copy link
Copy Markdown
Contributor

sorry, @slfan1989 just a very minor question, in this commit 3a41476 where you make changes, i notice you remove one tokio::time::sleep in line 221, but didn't replace with anything. If this case is not applicable for the helper, should we still keep it untouched?

@slfan1989
Copy link
Copy Markdown
Contributor Author

sorry, @slfan1989 just a very minor question, in this commit 3a41476 where you make changes, i notice you remove one tokio::time::sleep in line 221, but didn't replace with anything. If this case is not applicable for the helper, should we still keep it untouched?

@charlesdong1991 Thanks for checking this. You are right — this sleep is after flush() and is not part of the table/partition readiness waits covered by the helper. I restored it to keep this PR scoped to replacing readiness-related sleeps only.

Copy link
Copy Markdown
Contributor

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

@fresh-borzoni fresh-borzoni merged commit 9f69ae4 into apache:main May 28, 2026
10 checks passed
@slfan1989
Copy link
Copy Markdown
Contributor Author

@fresh-borzoni @charlesdong1991 Thank you very much for reviewing the code and helping merge it!

@fresh-borzoni
Copy link
Copy Markdown
Member

@slfan1989 The pleasure is all mine, thank you for the nice contribution 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[tests] replace fixed tokio::time::sleep workarounds with wait_for_table_ready helper

3 participants