Copilot commented on code in PR #3612:
URL: https://github.com/apache/fluss/pull/3612#discussion_r3564335081
##########
fluss-server/src/main/java/org/apache/fluss/server/storage/DiskUsageMonitor.java:
##########
@@ -137,23 +136,43 @@ public double getWriteLimitRatio() {
return writeLimitRatio;
}
+ public double getRecoverGap() {
+ return recoverGap;
+ }
+
public double getRecoverThreshold() {
return recoverThreshold;
}
/**
- * Dynamically updates the write-limit ratio and the derived recover
threshold. The new ratio
- * takes effect on the next {@link #runOnce()} invocation or {@link
#update(double)} call.
+ * Dynamically updates the write-limit ratio, recover gap and the derived
recover threshold. The
+ * new values take effect on the next {@link #runOnce()} invocation or
{@link #update(double)}
+ * call.
*
- * @param newRatio the new write-limit ratio, must be within (0.0, 1.0]
+ * @param newRatio the new write-limit ratio, must be within (recoverGap,
1.0]
+ * @param newRecoverGap the new recover gap, must be within (0.0,
writeLimitRatio)
*/
Review Comment:
The Javadoc parameter constraints reference the wrong identifiers (current
field names) which makes the contract ambiguous. These constraints should refer
to the new values (newRecoverGap/newRatio).
##########
website/docs/maintenance/configuration.md:
##########
@@ -67,22 +67,23 @@ during the Fluss cluster working.
## TabletServer
-| Option | Type | Default
| Description
|
-|--------------------------------------------------|------------|-----------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| tablet-server.id | Integer | (None)
| The id for the tablet server.
|
-| tablet-server.rack | String | (None)
| The rack for the TabletServer. This will be used in rack aware bucket
assignment for fault tolerance. Examples: `RACK1`, `cn-hangzhou-server10`
|
-| data.dir | String |
/tmp/fluss-data | This configuration controls the directory where Fluss will
store its data. The default value is /tmp/fluss-data
|
-| server.writer-id.expiration-time | Duration | 7d
| The time that the tablet server will wait without receiving any write
request from a client before expiring the related status. The default value is
7 days.
|
-| server.writer-id.expiration-check-interval | Duration | 10min
| The interval at which to remove writer ids that have expired due to
`server.writer-id.expiration-time passing. The default value is 10 minutes.
|
-| server.background.threads | Integer | 10
| The number of threads to use for various background processing tasks. The
default value is 10.
|
-| server.buffer.memory-size | MemorySize | 256mb
| The total bytes of memory the server can use, e.g, buffer write-ahead-log
rows.
|
-| server.buffer.page-size | MemorySize | 128kb
| Size of every page in memory buffers (`server.buffer.memory-size`).
|
-| server.buffer.per-request-memory-size | MemorySize | 16mb
| The minimum number of bytes that will be allocated by the writer rounded
down to the closest multiple of server.buffer.page-size. It must be greater
than or equal to server.buffer.page-size. This option allows to allocate memory
in batches to have better CPU-cached friendliness due to contiguous segments.
|
-| server.buffer.wait-timeout | Duration | 2^(63)-1ns
| Defines how long the buffer pool will block when waiting for segments.
|
-| tablet-server.controlled-shutdown.max-retries | Integer | 3
| Maximum number of attempts to transfer leadership before proceeding with
an unclean shutdown during a controlled shutdown procedure.
|
-| tablet-server.controlled-shutdown.retry-interval | Duration | 1000ms
| Time interval between retry attempts when trying to transfer leadership
during controlled shutdown.
|
-| server.data-disk.write-limit-ratio | Double | 0.85
| Reject writes when the tablet server data disk usage exceeds this ratio.
Writes resume after the usage drops below `(ratio - 0.10)`. The monitor reports
the maximum usage across all distinct file stores so that a single nearly-full
disk is never masked by other low-usage disks. Set to `1.0` to disable the
disk-usage protection entirely. The valid range is `(0.1, 1.0]`. This
configuration can be updated dynamically without server restart. |
-| server.data-disk.check-interval | Duration | 30s
| The interval at which the tablet server samples the local data disk usage
for the write-protection state machine. A shorter interval narrows the time
window during which writes can still flow in after the disk crosses the limit
ratio, at the cost of slightly more frequent `statvfs` calls (which are
in-memory and cheap). The default 30s is suitable for typical write workloads.
|
+| Option | Type | Default
| Description
|
+|--------------------------------------------------|------------|------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| tablet-server.id | Integer | (None)
| The id for the tablet server.
|
+| tablet-server.rack | String | (None)
| The rack for the TabletServer. This will be used in rack aware bucket
assignment for fault tolerance. Examples: `RACK1`, `cn-hangzhou-server10`
|
+| data.dir | String |
/tmp/fluss-data | This configuration controls the directory where Fluss will
store its data. The default value is /tmp/fluss-data
|
+| server.writer-id.expiration-time | Duration | 7d
| The time that the tablet server will wait without receiving any write
request from a client before expiring the related status. The default value is
7 days.
|
+| server.writer-id.expiration-check-interval | Duration | 10min
| The interval at which to remove writer ids that have expired due to
`server.writer-id.expiration-time passing. The default value is 10 minutes.
|
Review Comment:
The Markdown inline code span in this table row is missing a closing
backtick, so the rest of the row renders as code. It should read: due to
`server.writer-id.expiration-time` passing.
##########
fluss-server/src/main/java/org/apache/fluss/server/storage/LocalDiskManager.java:
##########
@@ -515,41 +529,52 @@ public DiskUsageMonitor getDiskUsageMonitor() {
}
// ------------------------------------------------------------------------
- // ServerReconfigurable: dynamic write-limit-ratio update
+ // ServerReconfigurable: dynamic disk write-limit update
// ------------------------------------------------------------------------
@Override
public void validate(Configuration newConfig) throws ConfigException {
double newRatio =
newConfig.get(ConfigOptions.SERVER_DATA_DISK_WRITE_LIMIT_RATIO);
- if (newRatio <= 0.1 || newRatio > 1.0) {
- throw new ConfigException(
- String.format(
- "Invalid %s: must be within (0.1, 1.0], but was
%s",
-
ConfigOptions.SERVER_DATA_DISK_WRITE_LIMIT_RATIO.key(), newRatio));
+ double newRecoverGap =
+
newConfig.get(ConfigOptions.SERVER_DATA_DISK_WRITE_LIMIT_RECOVER_GAP);
+ String validationError =
+ DiskWriteLimitConfigValidator.getValidationError(newRatio,
newRecoverGap);
+ if (validationError != null) {
+ throw new ConfigException(validationError);
}
}
@Override
public void reconfigure(Configuration newConfig) {
double newRatio =
newConfig.get(ConfigOptions.SERVER_DATA_DISK_WRITE_LIMIT_RATIO);
- if (Double.compare(newRatio, diskWriteLimitRatio) == 0) {
+ double newRecoverGap =
+
newConfig.get(ConfigOptions.SERVER_DATA_DISK_WRITE_LIMIT_RECOVER_GAP);
+ if (Double.compare(newRatio, diskWriteLimitRatio) == 0
Review Comment:
`reconfigure()` assumes `validate()` was enforced. If it is ever called with
an invalid config (e.g., when skipErrorConfig=true during ZK sync),
`diskUsageMonitor.updateWriteLimitConfig(...)` will throw an unchecked
IllegalArgumentException. Re-validating here and throwing a ConfigException
makes the failure mode consistent and prevents unexpected runtime exceptions
during dynamic config application.
--
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]
