szehon-ho commented on code in PR #17707: URL: https://github.com/apache/iceberg/pull/17707#discussion_r3858729061
########## docs/docs/spark-procedures.md: ########## @@ -410,6 +410,7 @@ Iceberg can compact data files in parallel using Spark with the `rewriteDataFile | `min-input-files` | 5 | Any file group with this number of files or more will be rewritten regardless of other criteria (the file group should have at least two files) | | `rewrite-all` | false | Force rewriting of all provided files overriding other options | | `max-file-group-size-bytes` | 107374182400 (100GB) | Largest amount of data that should be rewritten in a single file group. The entire rewrite operation is broken down into pieces based on partitioning and within partitions based on size into file-groups. This helps with breaking down the rewriting of very large partitions which may not be rewritable otherwise due to the resource constraints of the cluster. | +| `max-file-group-input-files` | Long.MAX_VALUE (unlimited) | Largest number of input files that should be rewritten in a single file group. This helps with breaking down the rewriting of very large partitions which may not be rewritable otherwise due to the resource constraints of the cluster. | Review Comment: Consider adding a note that this should be set to at least `min-input-files`. The cap is applied during bin-packing, so with `max-file-group-input-files=2` and the default `min-input-files=5`, `enoughInputFiles` can never be true and groups only survive on the size or delete-ratio predicates. Users who set this low can get a silent no-op, and nothing validates the two options against each other. Same applies to the new row in the `rewrite_position_delete_files` table. ########## docs/docs/spark-procedures.md: ########## @@ -549,6 +550,7 @@ Dangling deletes are always filtered out during rewriting. | `min-input-files` | 5 | Any file group exceeding this number of files will be rewritten regardless of other criteria | | `rewrite-all` | false | Force rewriting of all provided files overriding other options | | `max-file-group-size-bytes` | 107374182400 (100GB) | Largest amount of data that should be rewritten in a single file group. The entire rewrite operation is broken down into pieces based on partitioning and within partitions based on size into file-groups. This helps with breaking down the rewriting of very large partitions which may not be rewritable otherwise due to the resource constraints of the cluster. | +| `max-file-group-input-files` | Long.MAX_VALUE (unlimited) | Largest number of input files that should be rewritten in a single file group. This helps with breaking down the rewriting of very large partitions which may not be rewritable otherwise due to the resource constraints of the cluster. | Review Comment: Consider `9223372036854775807 (unlimited)` instead of `Long.MAX_VALUE (unlimited)`, here and in the `rewrite_data_files` table. Every other default in these tables is a literal value, including `delete-file-threshold` which spells out `2147483647`. Fine to leave as-is though, since `flink-maintenance.md` already uses `Long.MAX_VALUE` for this same option. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
