[Iceberg compaction] Rewrite planner transform - #39707
Conversation
eb91fac to
7327c47
Compare
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
7327c47 to
8d24582
Compare
ahmedabu98
changed the title
|
|
||
| String partitionPath = table.spec().partitionToPath(group.info().partition()); | ||
| partitionPaths.add(partitionPath); | ||
| int commitKey = 0; |
There was a problem hiding this comment.
Could you help me to understand why commitKey is hardcoded to 0 ? @ahmedabu98
There was a problem hiding this comment.
The code got updated since you reviewed. it's now a round robin commitKey = plannedGroupIndex % maxCommits
| List<FileScanTask> tasks, long splitSize) { | ||
| long effectiveSplitSize = Math.max(1L, splitSize); | ||
| return TableScanUtil.planTaskGroups( | ||
| tasks, effectiveSplitSize, /* lookback= */ 10, /* openFileCost= */ 0L); |
There was a problem hiding this comment.
With openFileCost=0, weight is bytes only, so the number of files per bin is unbounded.
Say 200,000 × 1 KB files in one partition, becomes one parent group, one bin, one subgroup rewritten by a single worker.
use a configurable openFileCost instead of 0 could be better. What do you think ?
There was a problem hiding this comment.
IIUC openFileCost is meant for scan planning. Here we're trying to make batches that will produce target output file sizes. In this case we're intentionally making it bytes-only. Otherwise factoring in the openFileCost per file may give us groups where the real output size is much smaller than the calculated one, because it's eaten up by the apparent weight from openFileCost.
The edge case you bring up could happen though. Maybe we should expose a "max files per subgroup" option
There was a problem hiding this comment.
Note Iceberg 1.11.0 added a max-file-group-input-files option: https://github.com/apache/iceberg/blob/dcd9feebbc46d6f64a25461654b83f3028f80971/core/src/main/java/org/apache/iceberg/actions/SizeBasedFileRewritePlanner.java#L110-L117
In our case, this would help control the parent group's file count
2 participants
Part of #39221
Adds a transform that plans the rewrite, and adds a configuration class defining rewrite options
Requires #39706
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.