jackye1995 commented on a change in pull request #2501: URL: https://github.com/apache/iceberg/pull/2501#discussion_r618033799
########## File path: api/src/main/java/org/apache/iceberg/actions/CompactDataFiles.java ########## @@ -0,0 +1,140 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.actions; + +import java.util.Map; +import org.apache.iceberg.actions.compaction.BinPack; +import org.apache.iceberg.expressions.Expression; + +public interface CompactDataFiles extends Action<CompactDataFiles, CompactDataFiles.Result> { + + /** + * Enable committing groups of chunks prior to the entire compaction completing. This will produce additional commits + * but allow for progress even if some chunks fail to commit. The default is false, which produces a single commit + * when all chunks have completed. + */ + String PARTIAL_PROGRESS_ENABLED = "partial_progress.enabled"; Review comment: I would like to revisit a bit of the decision about using this `max_commits`. The reason we decided to go with this was to not produce 1 commit per partition and results in millions of commits in some cases. But now I think about it, using max commits has the following 2 concerns: 1. the more partitions there are, the more partitions are compacted in a single commit, so the more likely it is for each commit to fail, which kind of defeats our purpose to make partial progress for tables with many partitions. 2. For tables with only a few partitions (e.g. region partitioned), it might be fine to have 1 commit per partition, which means I need to customize a different number input for each table I compact, which makes this config inflexible. So what about using something like a percentage? for example, `PARTIAL_PROGRESS_NEW_COMMIT_THRESHOLD`, which defines the percentage of partitions/chunks to be included in a single commit, so that for issue (2), user can set just generic numbers such as 100% for small tables, 10% for big tables, etc. To tackle (1), another option we can add is a lower bound, something like `PARTIAL_PROGRESS_MAX_CHUNKS_PER_COMMIT`, so that we guarantee each commit can still succeed with reasonable likelihood. When working together with other options, this value will override the MAX_COMMIT or percentage, so that when set, if one commit might include too many things, this config can act as a safeguard. -- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
