Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-33156][Test]Remove flakiness from tests in OperatorStateBackendTest.java #23464

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asha-boyapati
Copy link
Contributor

@asha-boyapati asha-boyapati commented Sep 26, 2023

What is the purpose of the change

This PR is similar to the following PR which was accepted: #23298

This PR makes the following tests stable:

org.apache.flink.runtime.state.OperatorStateBackendTest#testSnapshotRestoreSync
org.apache.flink.runtime.state.OperatorStateBackendTest#testSnapshotRestoreAsync

The tests are currently flaky because the order of elements returned by iterators is non-deterministic.

This PR fixes the flaky tests by making them independent of the order of elements returned by the iterators.

We detected this using the NonDex tool using the following commands:

mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex -pl flink-runtime -DnondexRuns=10 -Dtest=org.apache.flink.runtime.state.OperatorStateBackendTest#testSnapshotRestoreSync

mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex -pl flink-runtime -DnondexRuns=10 -Dtest=org.apache.flink.runtime.state.OperatorStateBackendTest#testSnapshotRestoreAsync

Please see the following Continuous Integration log that shows the flakiness:
https://github.com/asha-boyapati/flink/actions/runs/6193757385

Please see the following Continuous Integration log that shows that the flakiness is fixed by this change:
https://github.com/asha-boyapati/flink/actions/runs/6194044449

The Jira ticket associated with this PR is as follows:
https://issues.apache.org/jira/browse/FLINK-33156

Brief change log

This PR fixes the flaky tests by making them independent of the order of elements returned by the iterators.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

This change only modifies tests slightly and does not change any underlying code.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@flinkbot
Copy link
Collaborator

flinkbot commented Sep 26, 2023

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Contributor

@masteryhx masteryhx left a comment

Choose a reason for hiding this comment

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

Thanks for the pr. Please correct the title firstly: [FLINK-xxx][test] xxx

Could you use NonDex and manul verification to check all these problems together ?
I have seen so many cases, e.g. OperatorStateBackendTest#testRegisterStates, StateBackendMigrationTestBase#testKeyedListStateUpgrade

I think we should handle all these problems in this pr.

@asha-boyapati asha-boyapati changed the title Remove flakiness from tests in OperatorStateBackendTest.java [FLINK-33156][Test]Remove flakiness from tests in OperatorStateBackendTest.java Sep 28, 2023
@asha-boyapati
Copy link
Contributor Author

Thanks for your comments. I updated the title. I will look into your suggestion of finding and fixing similar problems together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants