[MINOR] Stop TestIcebergSync generating zero-record data files - #919
Open
slachiewicz wants to merge 1 commit into
Open
[MINOR] Stop TestIcebergSync generating zero-record data files#919slachiewicz wants to merge 1 commit into
slachiewicz wants to merge 1 commit into
Conversation
Iceberg's InclusiveMetricsEvaluator returns ROWS_CANNOT_MATCH for any file whose recordCount is 0, so such a file is pruned from every filtered scan. getDataFile drew recordCount from an unseeded Random over [0, 10000), so roughly one run in five thousand dropped a file that validateIcebergTable expected and failed with "expected: <2> but was: <1>". Do not fold this back into a plain nextInt: a data file with no records is not a fixture any of these tests mean to build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TestIcebergSyncfails intermittently withexpected: <2> but was: <1>, most recently on #912, where the change under review can't reach the test: it mocks both the schema extractor and the partition spec extractor.What is the purpose of the pull request
getDataFiledrawsrecordCountfrom an unseededRandomover[0, 10000), so it occasionally builds a data file with no records. Iceberg'sInclusiveMetricsEvaluatorreturnsROWS_CANNOT_MATCHfor any file whoserecordCountis 0, which prunes that file from every filtered scan. The tests that pass a filter expression tovalidateIcebergTablethen see one file fewer than they staged, and the file count assertion fails.The odds are about one run in five thousand per filtered test, which is why it reads as an unrelated red build rather than a reproducible failure.
fileSizeBytesis drawn the same way and is left alone. At roughly 10 KB a file, both files stay well under the 4 MB open file cost and the 128 MB split target, soplanTasksreturns a single combined scan task regardless of the draw.Brief change log
TestIcebergSync#getDataFiledrawsrecordCountfrom[1, 10000].Verify this pull request
This change is a test fixture fix and can be verified as follows:
./mvnw -pl xtable-core test -Dtest=TestIcebergSyncruns 9 tests, all green, on JDK 11.recordCountof 0 and leaving the others alone reproduces the CI failure exactly:TestIcebergSync.testNestedFieldPartitioning:680->validateIcebergTable:845 expected: <2> but was: <1>. Zeroing all three instead loses every file and fails one assertion earlier, at line 843.InclusiveMetricsEvaluator$MetricsEvalVisitor.evaliniceberg-api, which returnsROWS_CANNOT_MATCHwhenrecordCountis 0, before it reads any column statistics.This change was created with AI assistance.