You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/12/08 06:59:22 UTC
[GitHub] [iceberg] singhpk234 commented on a diff in pull request #6378: Spark: Extend Timeout During Partial Progress Rewrites
singhpk234 commented on code in PR #6378:
URL: https://github.com/apache/iceberg/pull/6378#discussion_r1042869209
##########
core/src/main/java/org/apache/iceberg/actions/RewriteDataFilesCommitManager.java:
##########
@@ -225,25 +225,40 @@ public void close() {
LOG.info("Closing commit service for {}", table);
committerService.shutdown();
+ boolean timeout = false;
+ int waitTime;
try {
// All rewrites have completed and all new files have been created, we are now waiting for
// the commit
- // pool to finish doing it's commits to Iceberg State. In the case of partial progress this
+ // pool to finish doing its commits to Iceberg State. In the case of partial progress this
// should
// have been occurring simultaneously with rewrites, if not there should be only a single
// commit operation.
- // In either case this should take much less than 10 minutes to actually complete.
- if (!committerService.awaitTermination(10, TimeUnit.MINUTES)) {
+ // We will wait 10 minutes plus 5 more minutes for each commit left to perform due to the
+ // time required for writing manifests
+ waitTime = 10 + (completedRewrites.size() / rewritesPerCommit) * 5;
+ if (!committerService.awaitTermination(waitTime, TimeUnit.MINUTES)) {
LOG.warn(
- "Commit operation did not complete within 10 minutes of the files being written. This may mean "
- + "that changes were not successfully committed to the the Iceberg table.");
+ "Commit operation did not complete within {} (10 + 5 * commitsRemaining) minutes of the all files "
+ + "being rewritten. This may mean that changes were not successfully committed to the the "
+ + "Iceberg catalog.",
Review Comment:
[doubt] should we say Iceberg table instead of iceberg catalog ?
##########
core/src/main/java/org/apache/iceberg/actions/RewriteDataFilesCommitManager.java:
##########
@@ -225,25 +225,40 @@ public void close() {
LOG.info("Closing commit service for {}", table);
committerService.shutdown();
+ boolean timeout = false;
+ int waitTime;
try {
// All rewrites have completed and all new files have been created, we are now waiting for
// the commit
- // pool to finish doing it's commits to Iceberg State. In the case of partial progress this
+ // pool to finish doing its commits to Iceberg State. In the case of partial progress this
// should
// have been occurring simultaneously with rewrites, if not there should be only a single
// commit operation.
- // In either case this should take much less than 10 minutes to actually complete.
- if (!committerService.awaitTermination(10, TimeUnit.MINUTES)) {
+ // We will wait 10 minutes plus 5 more minutes for each commit left to perform due to the
+ // time required for writing manifests
+ waitTime = 10 + (completedRewrites.size() / rewritesPerCommit) * 5;
Review Comment:
[doubt] is the 5 min per commit, coming from present defaults i.e commit.retry.num-retries = 4, commit.retry.max-wait-ms = 1 min ?
--
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