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

[HUDI-5358] Fix flaky tests in TestCleanerInsertAndCleanByCommits #7420

Merged

Conversation

yihua
Copy link
Contributor

@yihua yihua commented Dec 9, 2022

Change Logs

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x

with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.

This fix is required for CI to pass for #7396.

Impact

Only fixing flaky tests. The validation in cleaner tests is now more robust.

Risk level (write none, low medium or high below)

none

Documentation Update

N/A

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

Copy link
Contributor

@nsivabalan nsivabalan left a comment

Choose a reason for hiding this comment

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

Good find buddy!

@alexeykudinkin
Copy link
Contributor

Thanks for cleaning this up @yihua!

@hudi-bot
Copy link

hudi-bot commented Dec 9, 2022

CI report:

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

@alexeykudinkin
Copy link
Contributor

CI is green:

Screenshot 2022-12-09 at 2 03 04 PM

https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build/results?buildId=13588&view=results

@alexeykudinkin alexeykudinkin merged commit c2c0fa0 into apache:master Dec 9, 2022
@yihua yihua added priority:blocker release-0.12.2 Patches targetted for 0.12.2 labels Dec 12, 2022
nsivabalan pushed a commit that referenced this pull request Dec 13, 2022
)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
alexeykudinkin pushed a commit that referenced this pull request Dec 14, 2022
)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
fengjian428 pushed a commit to fengjian428/hudi that referenced this pull request Apr 5, 2023
…ache#7420)

This PR fixes the validation logic inside cleaner tests in TestCleanerInsertAndCleanByCommits.

In the tests, the KEEP_LATEST_COMMITS cleaner policy is used. This policy first figures out the earliest commit to retain based on the config of the number of retained commits (hoodie.cleaner.commits.retained). Then, for each file group, one more version before the earliest commit to retain is also kept from cleaning. The commit for the version can be different among file groups. For example, given the following commits, with x denoting a base file written for the corresponding commit and file group,

         c1  c2  c3  c4  c5
fg1:     x        x   x  x
fg2:          x   x   x  x
with hoodie.cleaner.commits.retained=3, no files are cleaned.

However, the current validation logic only statically picks the one commit before the earliest commit to retain in the Hudi timeline, i.e., c2 in the above example, for all file groups, marking the base file written by c1 for fg1 to be expected for cleaning, which does not match the KEEP_LATEST_COMMITS cleaner policy. The validation logic is fixed according to the expected cleaner behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:blocker release-0.12.2 Patches targetted for 0.12.2
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants