kevinjqliu commented on code in PR #13268:
URL: https://github.com/apache/iceberg/pull/13268#discussion_r2310834979
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -86,6 +86,19 @@ public class IcebergSinkConfig extends AbstractConfig {
private static final String COMMIT_TIMEOUT_MS_PROP =
"iceberg.control.commit.timeout-ms";
private static final int COMMIT_TIMEOUT_MS_DEFAULT = 30_000;
private static final String COMMIT_THREADS_PROP =
"iceberg.control.commit.threads";
+ private static final String COMMIT_MAX_RETRIES_PROP =
"iceberg.control.commit.max-retries";
+ private static final int COMMIT_MAX_RETRIES_DEFAULT = 3;
+ private static final String COMMIT_MIN_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.min-retry-wait-ms";
+ private static final int COMMIT_MIN_RETRY_WAIT_MS_DEFAULT = 100;
+ private static final String COMMIT_MAX_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.max-retry-wait-ms";
+ private static final int COMMIT_MAX_RETRY_WAIT_MS_DEFAULT = 60_000;
Review Comment:
different from docs
```
| iceberg.control.commit.max-retry-wait-ms | Maximum wait time in ms to
wait before retry, default is 300,000 (5 minutes)
|
```
also i prefer 5*60*1000
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -86,6 +86,19 @@ public class IcebergSinkConfig extends AbstractConfig {
private static final String COMMIT_TIMEOUT_MS_PROP =
"iceberg.control.commit.timeout-ms";
private static final int COMMIT_TIMEOUT_MS_DEFAULT = 30_000;
private static final String COMMIT_THREADS_PROP =
"iceberg.control.commit.threads";
+ private static final String COMMIT_MAX_RETRIES_PROP =
"iceberg.control.commit.max-retries";
Review Comment:
this is difference from the docs, its `iceberg.control.commit.max-retry` on
the doc side
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -86,6 +86,19 @@ public class IcebergSinkConfig extends AbstractConfig {
private static final String COMMIT_TIMEOUT_MS_PROP =
"iceberg.control.commit.timeout-ms";
private static final int COMMIT_TIMEOUT_MS_DEFAULT = 30_000;
private static final String COMMIT_THREADS_PROP =
"iceberg.control.commit.threads";
+ private static final String COMMIT_MAX_RETRIES_PROP =
"iceberg.control.commit.max-retries";
+ private static final int COMMIT_MAX_RETRIES_DEFAULT = 3;
+ private static final String COMMIT_MIN_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.min-retry-wait-ms";
+ private static final int COMMIT_MIN_RETRY_WAIT_MS_DEFAULT = 100;
+ private static final String COMMIT_MAX_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.max-retry-wait-ms";
+ private static final int COMMIT_MAX_RETRY_WAIT_MS_DEFAULT = 60_000;
+ private static final String COMMIT_TOTAL_RETRY_TIME_MS_PROP =
+ "iceberg.control.commit.total-retry-time-ms";
+ private static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 300_000; // 5
minutes
+ public static final String FAIL_ON_MAX_COMMIT_RETRIES =
"fail.on.max.commit.retries";
Review Comment:
nit: should this also have `iceberg.` prefix?
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -86,6 +86,19 @@ public class IcebergSinkConfig extends AbstractConfig {
private static final String COMMIT_TIMEOUT_MS_PROP =
"iceberg.control.commit.timeout-ms";
private static final int COMMIT_TIMEOUT_MS_DEFAULT = 30_000;
private static final String COMMIT_THREADS_PROP =
"iceberg.control.commit.threads";
+ private static final String COMMIT_MAX_RETRIES_PROP =
"iceberg.control.commit.max-retries";
+ private static final int COMMIT_MAX_RETRIES_DEFAULT = 3;
+ private static final String COMMIT_MIN_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.min-retry-wait-ms";
+ private static final int COMMIT_MIN_RETRY_WAIT_MS_DEFAULT = 100;
+ private static final String COMMIT_MAX_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.max-retry-wait-ms";
+ private static final int COMMIT_MAX_RETRY_WAIT_MS_DEFAULT = 60_000;
+ private static final String COMMIT_TOTAL_RETRY_TIME_MS_PROP =
+ "iceberg.control.commit.total-retry-time-ms";
+ private static final int COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT = 300_000; // 5
minutes
Review Comment:
these are not in the docs
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/IcebergSinkConfig.java:
##########
@@ -86,6 +86,19 @@ public class IcebergSinkConfig extends AbstractConfig {
private static final String COMMIT_TIMEOUT_MS_PROP =
"iceberg.control.commit.timeout-ms";
private static final int COMMIT_TIMEOUT_MS_DEFAULT = 30_000;
private static final String COMMIT_THREADS_PROP =
"iceberg.control.commit.threads";
+ private static final String COMMIT_MAX_RETRIES_PROP =
"iceberg.control.commit.max-retries";
+ private static final int COMMIT_MAX_RETRIES_DEFAULT = 3;
+ private static final String COMMIT_MIN_RETRY_WAIT_MS_PROP =
+ "iceberg.control.commit.min-retry-wait-ms";
+ private static final int COMMIT_MIN_RETRY_WAIT_MS_DEFAULT = 100;
Review Comment:
this default is different too
```
| iceberg.control.commit.min-retry-wait-ms | Minimum wait time in ms to
wait before retry, default is 60,000 (60 sec)
|
```
--
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]
