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 2021/05/17 07:25:55 UTC
[GitHub] [iceberg] coolderli opened a new pull request #2596: Core: Add check-status config to TableProperties.
coolderli opened a new pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596
Currently, the minWaitMs and maxWaitMs are 1000 by default, we should add the configs of check-status to TableProperties.
I don't know if the default values I configured are appropriate.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#discussion_r637226771
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -36,9 +36,18 @@ private TableProperties() {
public static final String COMMIT_TOTAL_RETRY_TIME_MS = "commit.retry.total-timeout-ms";
public static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 1800000; // 30 minutes
- public static final String COMMIT_NUM_STATUS_CHECKS = "commit.num-status-checks";
+ public static final String COMMIT_NUM_STATUS_CHECKS = "commit.status-checks.num-retries";
Review comment:
The other properties are in `commit.status-check`, not `commit.status-checks`. Could you remove the extra `s`?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] coolderli commented on pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
coolderli commented on pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#issuecomment-845836626
@rdblue @aokolnychyi I have checked the latest tag [0.11.1,](https://github.com/apache/iceberg/blob/apache-iceberg-0.11.1/core/src/main/java/org/apache/iceberg/TableProperties.java) the `commit.num-status-checks` has not been released yet. I have renamed it to `commit.status-checks.num-retries`.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#issuecomment-846285011
Merged! Can you also update the table properties docs, @coolderli?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#issuecomment-844575077
This looks good to me, but we should clean up the property name if it hasn't been in a release yet.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#discussion_r637227048
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -36,9 +36,18 @@ private TableProperties() {
public static final String COMMIT_TOTAL_RETRY_TIME_MS = "commit.retry.total-timeout-ms";
public static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 1800000; // 30 minutes
- public static final String COMMIT_NUM_STATUS_CHECKS = "commit.num-status-checks";
+ public static final String COMMIT_NUM_STATUS_CHECKS = "commit.status-checks.num-retries";
Review comment:
Nevermind, I could do it to the PR through github. I'll merge when tests are passing.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] coolderli commented on pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
coolderli commented on pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#issuecomment-852787663
@rdblue https://github.com/apache/iceberg/pull/2661
I have updated the docs.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#discussion_r635656148
##########
File path: core/src/main/java/org/apache/iceberg/TableProperties.java
##########
@@ -39,6 +39,15 @@ private TableProperties() {
public static final String COMMIT_NUM_STATUS_CHECKS = "commit.num-status-checks";
public static final int COMMIT_NUM_STATUS_CHECKS_DEFAULT = 3;
+ public static final String COMMIT_STATUS_CHECKS_MIN_WAIT_MS = "commit.status-check.min-wait-ms";
Review comment:
@coolderli, @aokolnychyi, has `commit.num-status-checks` been released yet?
If not, then we should rename it to mirror the `commit.retry.*` settings: `commit.status-check.num-retries`
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] coolderli commented on pull request #2596: Core: Add check-status config to TableProperties.
Posted by GitBox <gi...@apache.org>.
coolderli commented on pull request #2596:
URL: https://github.com/apache/iceberg/pull/2596#issuecomment-846328080
@rdblue OK, I will add the document as soon as possible
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org