You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "swuferhong (via GitHub)" <gi...@apache.org> on 2026/07/08 03:20:25 UTC
[PR] [server] Add configurable disk write-limit recover gap [fluss]
swuferhong opened a new pull request, #3612:
URL: https://github.com/apache/fluss/pull/3612
<!--
*Thank you very much for contributing to Fluss - we are happy that you want to help us improve Fluss. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
## Contribution Checklist
- Make sure that the pull request corresponds to a [GitHub issue](https://github.com/apache/fluss/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no issue.
- Name the pull request in the format "[component] Title of the pull request", where *[component]* should be replaced by the name of the component being changed. Typically, this corresponds to the component label assigned to the issue (e.g., [kv], [log], [client], [flink], [rust], [python], [c++], [elixir]). Skip *[component]* if you are unsure about which is the best component.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes.
- Each pull request should address only one issue, not mix up code from multiple issues.
- **Generative AI disclosure:** Indicate whether generative AI tools were used in authoring this PR. If yes, specify the tool below.
- [ ] No generative AI tools used
- [ ] Yes (please specify the tool below)
**(The sections below can be removed for hotfixes or typos)**
-->
<!--
Generated-by: [Tool Name and Version] following [the guidelines](https://github.com/apache/fluss/blob/main/AGENTS.md)
-->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #3611
Parameterize the `DiskUsageMonitor` recover gap with `server.data-disk.write-limit-recover-gap` and set its default value to `0.05`.
The recover gap now supports dynamic updates together with `server.data-disk.write-limit-ratio`. Startup and dynamic config validation now `enforce 0 < recover-gap < write-limit-ratio <= 1.0`.
<!-- What is the purpose of the change -->
### Brief change log
<!-- Please describe the changes made in this pull request and explain how they address the issue -->
### Tests
<!-- List UT and IT cases to verify this change -->
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Add configurable disk write-limit recover gap [fluss]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Add configurable disk write-limit recover gap [fluss]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3612:
URL: https://github.com/apache/fluss/pull/3612#discussion_r3563040411
##########
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 inline code span in this table row is missing a closing backtick, which will break Markdown rendering for the remainder of the cell (and potentially the table). Close the backtick around the config key.
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Add configurable disk write-limit recover gap [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong merged PR #3612:
URL: https://github.com/apache/fluss/pull/3612
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Add configurable disk write-limit recover gap [fluss]
Posted by "swuferhong (via GitHub)" <gi...@apache.org>.
swuferhong commented on PR #3612:
URL: https://github.com/apache/fluss/pull/3612#issuecomment-4954107753
@wuchong comments addressed. PTAL, thanks.
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [server] Add configurable disk write-limit recover gap [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on code in PR #3612:
URL: https://github.com/apache/fluss/pull/3612#discussion_r3566480852
##########
fluss-common/src/main/java/org/apache/fluss/config/ConfigOptions.java:
##########
@@ -385,10 +385,24 @@ public class ConfigOptions {
.doubleType()
.defaultValue(0.85)
.withDescription(
- "Reject writes when the tablet server data disk usage exceeds this ratio. "
- + "Writes resume after the usage drops below (ratio - 0.10). "
+ "Reject writes when the tablet server data disk usage reaches this ratio. "
+ + "Writes resume after the usage drops below "
+ + "(server.data-disk.write-limit-ratio - "
+ + "server.data-disk.write-limit-recover-gap). "
+ "Set to 1.0 to disable the disk-usage protection entirely. "
- + "The valid range is (0.1, 1.0].");
+ + "The valid range is "
+ + "(server.data-disk.write-limit-recover-gap, 1.0].");
+
+ public static final ConfigOption<Double> SERVER_DATA_DISK_WRITE_LIMIT_RECOVER_GAP =
+ key("server.data-disk.write-limit-recover-gap")
Review Comment:
Could we consider renaming these configuration keys and expressing both values as absolute watermarks?
Currently, we have:
- `server.data-disk.write-limit-ratio`
- `server.data-disk.write-limit-recover-gap`
Combining an absolute ratio with a relative gap requires users to calculate the actual recovery threshold themselves (`limit-ratio - recover-gap`). It also makes validation and dynamic updates harder to reason about.
I suggest using two mirrored thresholds that map directly to the write-protection state transitions:
- **`server.data-disk.write-limit-ratio`** — for example, `0.85`, at which write protection is activated.
- **`server.data-disk.write-recover-ratio`** — for example, `0.80`, at which write protection is deactivated.
The relationship then becomes straightforward:
```text
0 < write-recover-ratio < write-limit-ratio <= 1.0
```
--
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@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org