You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Baunsgaard (via GitHub)" <gi...@apache.org> on 2026/06/06 12:30:56 UTC

[PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Baunsgaard opened a new pull request, #16696:
URL: https://github.com/apache/iceberg/pull/16696

   ## What
   
   `RandomData` (Spark test helper) sized every generated list and map with
   `random.nextInt(20)`. Because the bound is applied at *every* nesting level, it
   multiplies for deeply-nested schemas. The worst case is
   `AvroDataTestBase.testMixedTypes`, which embeds the full ~19-field primitive
   struct two-to-three levels deep across five fields — each run generates well
   over a million leaf values, so the test cost is dominated by random-data volume
   rather than the read/write code paths being exercised.
   
   This replaces the hard-coded `20` with a named constant
   `MAX_COLLECTION_SIZE = 10` in the Spark 3.5 / 4.0 / 4.1 test copies.
   
   ## Why
   
   `testMixedTypes` is the single most expensive test method in the
   `iceberg-spark` core suite, appearing at the top of every format read/write
   test class. The collection size has no bearing on coverage — the schemas,
   types, and nesting structures under test are identical regardless of how many
   elements each collection holds — so this is pure scaffolding overhead.
   
   ## Impact
   
   Measured locally (JDK 17, Spark 3.5 core), `testMixedTypes` per class,
   single-threaded:
   
   | Class | before | after |
   |---|---:|---:|
   | TestAvroDataFrameWrite | 24.1s | 7.8s |
   | TestParquetDataFrameWrite | 20.0s | 3.6s |
   | TestORCDataFrameWrite | 19.9s | 3.4s |
   | TestParquetScan | 17.7s | 2.6s |
   | TestParquetVectorizedScan | 17.5s | 2.3s |
   | TestAvroScan | 17.5s | 2.4s |
   
   Collections still hold up to nine elements, preserving data variety.
   
   ## Testing
   
   `./gradlew :iceberg-spark:iceberg-spark-3.5_2.13:test` — **5,084 tests, 0
   failures** (identical pass/skip counts to before the change).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3387976408


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -178,7 +183,7 @@ private boolean isNull() {
 
     @Override
     public Object list(Types.ListType list, Supplier<Object> elementResult) {
-      int numElements = random.nextInt(20);
+      int numElements = random.nextInt(MAX_COLLECTION_SIZE);

Review Comment:
   Okay, i added an extra line comment saying that we can have empty collections. Good point!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #16696:
URL: https://github.com/apache/iceberg/pull/16696


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#issuecomment-4639153032

   Locally (Spark 3.5 core, JDK 17, single-threaded),  drops from
   ~17–24s to ~2–8s per class (3–7×), with identical pass/skip counts.
   
   However, on CI the effect is small and within runner noise (~±10%): core jobs 
   but on average ~2% to ~5% faster.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3387885301


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -178,7 +183,7 @@ private boolean isNull() {
 
     @Override
     public Object list(Types.ListType list, Supplier<Object> elementResult) {
-      int numElements = random.nextInt(20);
+      int numElements = random.nextInt(MAX_COLLECTION_SIZE);

Review Comment:
   Minor, since the comment already explains the multiplicative behavior: the range stays `[0, MAX_COLLECTION_SIZE)`, so empty collections are still possible and now show up at 1/10 instead of 1/20. Not a problem — empty collections are legal — but worth a half-line in the comment noting zero-length is in range so nobody reads the cap as a minimum.
   
   (Same four call sites in v4.0 and v4.1.)



##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   `nextInt(MAX_COLLECTION_SIZE)` is exclusive, so this actually caps collections at 9 elements, not 10 — the name reads like "max 10" and that's a trap for the next person who reaches for it. I'd either rename to something like `COLLECTION_SIZE_BOUND` to signal it's an exclusive bound, or bump the value to 11 if 10 elements is genuinely the intent. wdyt?
   
   (Same constant in v4.0 and v4.1.)



##########
spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -182,7 +187,7 @@ private boolean isNull() {
 
     @Override
     public Object list(Types.ListType list, Supplier<Object> elementResult) {
-      int numElements = random.nextInt(20);
+      int numElements = random.nextInt(MAX_COLLECTION_SIZE);

Review Comment:
   Same change applied identically here and in v4.1 — comment text is byte-for-byte consistent across all three copies, which is good. No drift to worry about.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3368006191


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   I see MAX_ENTRIES being used as the naming in [RandomGenericData.java](https://github.com/apache/iceberg/blob/4057f59dbee6878b3986f7cc39b1f30a59ee9ee3/data/src/test/java/org/apache/iceberg/data/RandomGenericData.java#L173).
   `MAX_COLLECTION_SIZE` does seem like a more apt name though but ok to keep MAX_ENTRIES as well to be consistent.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3368058704


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   The number before was 20, and reusing the name MAX_ENTRIES might be less clear, especially since we are working with collections specifically here. 
   
   Happy to change it if there are some other prefference.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#issuecomment-4710139906

   Thanks again for the PR @Baunsgaard and thank you @nssalian @laskoviymishka for the reviews! 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3387971828


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   We can change it to `_BOUND` this makes sense to me. 
   
   However, explicitly saying max 9, is a bit too much IMO, since the comment starts with `Exclusive upper bound` which exactly means what we intend to communicate. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3388038565


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   let's then keep it just as `_BOUND`, it's 



##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   let's then keep it just as `_BOUND`, it's enough here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#issuecomment-4671584573

   As a orthogonal followup i also have this PR: https://github.com/apache/iceberg/pull/16740 It gives another 8% if you want to take a look @laskoviymishka  and @nssalian ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark: Cap RandomData collection size to speed up nested tests [iceberg]

Posted by "Baunsgaard (via GitHub)" <gi...@apache.org>.
Baunsgaard commented on code in PR #16696:
URL: https://github.com/apache/iceberg/pull/16696#discussion_r3388166806


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/data/RandomData.java:
##########
@@ -52,6 +52,11 @@ public class RandomData {
   // Default percentage of number of values that are null for optional fields
   public static final float DEFAULT_NULL_PERCENTAGE = 0.05f;
 
+  // Exclusive upper bound on the number of elements generated for each list/map.
+  // Applied per nesting level, so deeply-nested schemas multiply quickly; keep
+  // this small enough to avoid combinatorial blow-up in heavily-nested tests.
+  private static final int MAX_COLLECTION_SIZE = 10;

Review Comment:
   Done! Changed to `COLLECTION_SIZE_BOUND`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org