mxm commented on code in PR #13832:
URL: https://github.com/apache/iceberg/pull/13832#discussion_r2281574664
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/maintenance/api/FlinkMaintenanceConfig.java:
##########
@@ -75,6 +103,11 @@ public long rateLimit() {
.parse();
}
+ /**
+ * Gets the parallelism value for maintenance tasks.
+ *
+ * @return The parallelism for maintenance tasks
+ */
Review Comment:
Same here, I would remove the `@return` line.
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/maintenance/api/FlinkMaintenanceConfig.java:
##########
@@ -31,27 +31,50 @@ public class FlinkMaintenanceConfig {
public static final String PREFIX = "flink-maintenance.";
+ // Configuration for lock check delay (in seconds)
+ // This setting controls the delay between each lock check during the
rewrite operation
public static final String LOCK_CHECK_DELAY = PREFIX +
"lock-check-delay-seconds";
public static final ConfigOption<Long> LOCK_CHECK_DELAY_OPTION =
ConfigOptions.key(LOCK_CHECK_DELAY)
.longType()
- .defaultValue(TableMaintenance.LOCK_CHECK_DELAY_SECOND_DEFAULT);
+ .defaultValue(TableMaintenance.LOCK_CHECK_DELAY_SECOND_DEFAULT)
+ .withDescription(
+ "The delay time (in seconds) between each lock check during the
rewrite operation.");
+ // Configuration for parallelism
+ // This setting controls the parallelism level for the maintenance tasks,
+ // determining how many tasks can run concurrently
public static final String PARALLELISM = PREFIX + "parallelism";
public static final ConfigOption<Integer> PARALLELISM_OPTION =
-
ConfigOptions.key(PARALLELISM).intType().defaultValue(ExecutionConfig.PARALLELISM_DEFAULT);
+ ConfigOptions.key(PARALLELISM)
+ .intType()
+ .defaultValue(ExecutionConfig.PARALLELISM_DEFAULT)
+ .withDescription(
+ "The parallelism level for the maintenance task. "
+ + "Determines how many tasks can run concurrently.");
+ // Configuration for rate limit (in seconds)
+ // This setting controls the rate at which maintenance operations can occur,
+ // limiting the number of operations per second
Review Comment:
Same here, do we need this comment with the description below?
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/maintenance/api/FlinkMaintenanceConfig.java:
##########
@@ -66,6 +89,11 @@ public FlinkMaintenanceConfig(
this.confParser = new FlinkConfParser(table, writeOptions, readableConfig);
}
+ /**
+ * Gets the rate limit value (in seconds) for maintenance operations.
+ *
+ * @return The rate limit for maintenance operations
+ */
Review Comment:
NIT: I find the `@return` a bit too verbose.
```suggestion
/**
* Gets the rate limit value (in seconds) for maintenance operations.
*/
```
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/maintenance/api/FlinkMaintenanceConfig.java:
##########
@@ -31,27 +31,50 @@ public class FlinkMaintenanceConfig {
public static final String PREFIX = "flink-maintenance.";
+ // Configuration for lock check delay (in seconds)
+ // This setting controls the delay between each lock check during the
rewrite operation
Review Comment:
With the description below, should we remove this comment?
--
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]
