Skip to content

Core: Skip testAddManyFilesWithConsistentOrdering if WORKER_THREAD_POOL_SIZE < 3#16506

Merged
stevenzwu merged 1 commit into
apache:mainfrom
ebyhr:ebi/test-merge-append-test
May 22, 2026
Merged

Core: Skip testAddManyFilesWithConsistentOrdering if WORKER_THREAD_POOL_SIZE < 3#16506
stevenzwu merged 1 commit into
apache:mainfrom
ebyhr:ebi/test-merge-append-test

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented May 21, 2026

The test fails if WORKER_THREAD_POOL_SIZE < 3.

Expected size: 3 but was: 2 in:
[GenericManifestFile{content=DATA, path=/var/folders/9s/_zwn4r_n2_9bp0krllp1pl3c0000gp/T/junit-2351142152275179508/metadata/4d4b563b-9372-4561-b141-f9434d3e5473-m1.avro, length=1517389, partition_spec_id=0, added_snapshot_id=1, added_data_files_count=15000, added_rows_count=1507481968, existing_data_files_count=0, existing_rows_count=0, deleted_data_files_count=0, deleted_rows_count=0, partitions=[GenericPartitionFieldSummary{contains_null=false, contains_nan=false, lower_bound=[0, 0, 0, 0], upper_bound=[1, 0, 0, 0]}], key_metadata=null, sequence_number=0, min_sequence_number=0, first_row_id=null},
    GenericManifestFile{content=DATA, path=/var/folders/9s/_zwn4r_n2_9bp0krllp1pl3c0000gp/T/junit-2351142152275179508/metadata/4d4b563b-9372-4561-b141-f9434d3e5473-m0.avro, length=1516450, partition_spec_id=0, added_snapshot_id=1, added_data_files_count=15000, added_rows_count=1507482102, existing_data_files_count=0, existing_rows_count=0, deleted_data_files_count=0, deleted_rows_count=0, partitions=[GenericPartitionFieldSummary{contains_null=false, contains_nan=false, lower_bound=[0, 0, 0, 0], upper_bound=[1, 0, 0, 0]}], key_metadata=null, sequence_number=0, min_sequence_number=0, first_row_id=null}]
java.lang.AssertionError: 
Expected size: 3 but was: 2 in:
[GenericManifestFile{content=DATA, path=/var/folders/9s/_zwn4r_n2_9bp0krllp1pl3c0000gp/T/junit-2351142152275179508/metadata/4d4b563b-9372-4561-b141-f9434d3e5473-m1.avro, length=1517389, partition_spec_id=0, added_snapshot_id=1, added_data_files_count=15000, added_rows_count=1507481968, existing_data_files_count=0, existing_rows_count=0, deleted_data_files_count=0, deleted_rows_count=0, partitions=[GenericPartitionFieldSummary{contains_null=false, contains_nan=false, lower_bound=[0, 0, 0, 0], upper_bound=[1, 0, 0, 0]}], key_metadata=null, sequence_number=0, min_sequence_number=0, first_row_id=null},
    GenericManifestFile{content=DATA, path=/var/folders/9s/_zwn4r_n2_9bp0krllp1pl3c0000gp/T/junit-2351142152275179508/metadata/4d4b563b-9372-4561-b141-f9434d3e5473-m0.avro, length=1516450, partition_spec_id=0, added_snapshot_id=1, added_data_files_count=15000, added_rows_count=1507482102, existing_data_files_count=0, existing_rows_count=0, deleted_data_files_count=0, deleted_rows_count=0, partitions=[GenericPartitionFieldSummary{contains_null=false, contains_nan=false, lower_bound=[0, 0, 0, 0], upper_bound=[1, 0, 0, 0]}], key_metadata=null, sequence_number=0, min_sequence_number=0, first_row_id=null}]
	at org.apache.iceberg.TestMergeAppend.testAddManyFilesWithConsistentOrdering(TestMergeAppend.java:110)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

Comment thread core/src/test/java/org/apache/iceberg/TestMergeAppend.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@ebyhr ebyhr marked this pull request as draft May 22, 2026 00:06
@ebyhr ebyhr force-pushed the ebi/test-merge-append-test branch from 0be6550 to 9201f40 Compare May 22, 2026 00:50
@ebyhr ebyhr changed the title Core: Set worker num in testAddManyFilesWithConsistentOrdering Core: Reduce multiplier to 2 in testAddManyFilesWithConsistentOrdering May 22, 2026
@ebyhr ebyhr marked this pull request as ready for review May 22, 2026 03:28
@stevenzwu
Copy link
Copy Markdown
Contributor

I also ran into the same problem.

We should probably fix this problem as part of this PR, which can explicitly control the writer thread pool and parallelism
#16108

cc @dramaticlly

public void testAddManyFilesWithConsistentOrdering() {
assertThat(listManifestFiles()).as("Table should start empty").isEmpty();

int multiplier = 3;
Copy link
Copy Markdown
Contributor

@stevenzwu stevenzwu May 22, 2026

Choose a reason for hiding this comment

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

let's just change the assumeThat check to 3 and leave other code unchanged. We can merge this small change quickly to fix the flakiness.

I mentioned the proper long term fix in another comment.

Copy link
Copy Markdown
Contributor

@dramaticlly dramaticlly May 22, 2026

Choose a reason for hiding this comment

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

Do we only run into this if the build only have 2 cores available?

If so we can update as Steven suggested as

    int multiplier = 3;
    assumeThat(ThreadPools.WORKER_THREAD_POOL_SIZE).isGreaterThanOrEqualTo(multiplier);

verified with ICEBERG_WORKER_NUM_THREADS=2 ./gradlew :iceberg-core:test --tests "org.apache.iceberg.TestMergeAppend" locally

Also plan to update this test with provided writeManifestWith pool so that we can have deterministic behavior regardless of core available in the build instance

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Reverted the multiplier change, and just added assumeThat.

@ebyhr ebyhr force-pushed the ebi/test-merge-append-test branch from 9201f40 to faed6bf Compare May 22, 2026 20:47
@ebyhr ebyhr changed the title Core: Reduce multiplier to 2 in testAddManyFilesWithConsistentOrdering Core: Skip testAddManyFilesWithConsistentOrdering if WORKER_THREAD_POOL_SIZE < 3 May 22, 2026
@ebyhr ebyhr force-pushed the ebi/test-merge-append-test branch from faed6bf to 08c98f5 Compare May 22, 2026 20:48
@ebyhr ebyhr force-pushed the ebi/test-merge-append-test branch from 08c98f5 to 6813041 Compare May 22, 2026 20:49
@stevenzwu stevenzwu merged commit 99e451a into apache:main May 22, 2026
53 checks passed
@stevenzwu
Copy link
Copy Markdown
Contributor

merged this. thanks @ebyhr for identifying and fixing the problem, and others for the review.

@dramaticlly we should include the proper fix for this test in the write thread pool PR.

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.

4 participants