Skip to content

Conversation

@zabetak
Copy link
Member

@zabetak zabetak commented Sep 23, 2025

Why are the changes needed?

Flakiness

Does this PR introduce any user-facing change?

No

How was this patch tested?

 mvn test -Dtest=TestIcebergCliDriver -Dqfile=query_iceberg_metadata_of_unpartitioned_table.q
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.hadoop.hive.cli.TestIcebergCliDriver
[WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 57.24 s -- in org.apache.hadoop.hive.cli.TestIcebergCliDriver
[INFO] 
[INFO] Results:
[INFO] 
[WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1
[INFO] 
[INFO] ------------------------------------------------------------------------

@sonarqubecloud
Copy link

@Aggarwal-Raghav
Copy link
Contributor

LGTM +1

Copy link
Contributor

@zhangbutao zhangbutao left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@zabetak zabetak merged commit c3f53bf into apache:master Sep 24, 2025
4 checks passed
@zabetak zabetak deleted the HIVE-29201-disable branch September 24, 2025 16:43
@deniskuzZ
Copy link
Member

@zabetak, @Aggarwal-Raghav, @zhangbutao could you please explain why test was disabled as flaky. Was there at least some investigation conducted, like flaky build check?
This test was added 2 years ago and all of the sudden became flaky? Maybe there is some other reason..

@zhangbutao
Copy link
Contributor

Apologies, I haven't looked into the root cause of this test instability carefully. But from what I recall, this test has always been prone to failure.

Therefore, I think before HIVE-29201 is resolved, we can temporarily disable this test on the main branch to prevent other new PRs from failing due to this test.

Additionally, I don't intend to disable this test permanently, as it relates to the important functionality of Iceberg metadata queries. Once the cause of the failure is identified, we can re-enable it under HIVE-29201. If time permits, I will also help investigate HIVE-29201. :)

@Aggarwal-Raghav
Copy link
Contributor

@deniskuzZ , please check the run:
https://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-6097/1/pipeline/421

I personally haven't investigated whether test is flaky or not but I assumed it was as the PR #6075 was already there.

Attaching screenshot as well:
Screenshot 2025-09-26 at 2 33 06 PM

@zabetak
Copy link
Member Author

zabetak commented Sep 26, 2025

@deniskuzZ I haven't explicitly run a flaky run but if you check the builds for many recent PRs they contain this failure. Moreover, this is not new and it appeared also in builds many months ago. Apart from that the description in HIVE-29201 clearly states some reasons for which the tests is flaky so I think there are enough evidence to disable it.

@deniskuzZ
Copy link
Member

deniskuzZ commented Sep 26, 2025

@deniskuzZ I haven't explicitly run a flaky run but if you check the builds for many recent PRs they contain this failure. Moreover, this is not new and it appeared also in builds many months ago. Apart from that the description in HIVE-29201 clearly states some reasons for which the tests is flaky so I think there are enough evidence to disable it.

@zabetak, the test don’t fail when run multiple times, so it's not flaky. If a library upgrade impacted the sorting of masked columns, that points to a core test framework issue, not a valid reason to disable the test. I think this warrants a discussion, and I find your decision questionable, especially since others were actively verifying this

@zabetak
Copy link
Member Author

zabetak commented Sep 26, 2025

@deniskuzZ I see that we disagree on some aspects on how we should approach this instability but now this is PR is merged so let's try to focus on the next steps.

If you need help on reviewing the other pending PRs about HIVE-29201 let me know and I will join the discussion. If you feel that this disablement needs to be reverted immediately let me know and I will be happy to do that as well.

@deniskuzZ
Copy link
Member

deniskuzZ commented Sep 30, 2025

@thomasrebele test infra fix has been merged (https://issues.apache.org/jira/browse/HIVE-29226), so we can re-enable the test now.
My point was that we shouldn’t mark a test as flaky and disable it without proper investigation.
The flaky-check run passed successfully: https://ci.hive.apache.org/job/hive-flaky-check/897/; the actual issue was with how masking and result sorting were applied.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants