You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "gyang94 (via GitHub)" <gi...@apache.org> on 2026/07/27 02:34:52 UTC
[PR] [server] feat: support table.log.local-ttl [fluss]
gyang94 opened a new pull request, #3778:
URL: https://github.com/apache/fluss/pull/3778
<!--
*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 #xxx
<!-- 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] Support independent local log TTL [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on code in PR #3778:
URL: https://github.com/apache/fluss/pull/3778#discussion_r3774664451
##########
fluss-server/src/main/java/org/apache/fluss/server/log/LogManager.java:
##########
@@ -502,6 +507,12 @@ private LogTablet loadLog(
return logTablet;
}
+ private long effectiveLocalLogTtlMs(long logTtlMs, long localLogTtlMs) {
Review Comment:
Could we move the effective local-TTL resolution into `LogTablet` and make every construction and hot-update path pass both `logTtlMs` and `localLogTtlMs` explicitly? The background is that these two values have different configuration meanings: `table.log.ttl` remains authoritative for local cleanup when remote tiering is disabled, while `table.log.local-ttl` controls local cleanup when tiering is enabled and falls back to `table.log.ttl` when it is not configured. Therefore the runtime value used by local cleanup is always `remoteLogEnabled ? localLogTtlMs : logTtlMs`.
The current patch resolves that runtime value in `LogManager` for normal creation and disk recovery, but the same policy is duplicated in `ReplicaManager#updateReplicaTableConfig()` for hot updates, after which `Replica#updateLocalLogTtlMs()` and `LogTablet#updateLocalLogTtlMs()` receive only the already-resolved value. The testing `LogTablet#create()` overload also passes `getLocalLogTTLMs()` directly. This makes the effective-value policy distributed across several call sites: a new construction or update path can bypass it, and the creation and hot-update implementations can drift independently. It also leaves `LogTablet.localLogTtlMs` ambiguous because the field actually stores the runtime effective TTL rather than necessarily the configured local TTL.
I suggest using the following ownership model:
1. Keep `TABLE_LOG_LOCAL_TTL` without a fixed default and let `TableConfig#getLocalLogTTLMs()` implement only the configuration fallback to `table.log.ttl`.
2. Remove `LogManager#effectiveLocalLogTtlMs()`. Every `LogTablet` constructor/factory call, including the normal creation path, disk recovery path, and testing overload, should pass both `logTtlMs` and `localLogTtlMs` unchanged. `LogManager#getOrCreateLog()` can continue returning an existing entry directly; no additional reconciliation is needed there.
3. Let `LogTablet` keep `remoteLogEnabled` and a single `volatile long effectiveLocalLogTtlMs`. Its constructor should initialize that field through one private `effectiveLocalLogTtlMs(logTtlMs, localLogTtlMs)` helper, and all expiration checks should use this field.
4. Replace `updateLocalLogTtlMs(long)` with `updateLogTtls(long logTtlMs, long localLogTtlMs)`, and have it call the same helper before storing the new effective value. This guarantees that construction and hot update use exactly the same rule.
5. For hot updates, let `ReplicaManager#updateReplicaTableConfig()` store `TableConfig` directly in a `Map<Long, TableConfig>`, then use its existing single pass over online replicas to call `replica.updateLogTtls(tableConfig.getLogTTLMs(), tableConfig.getLocalLogTTLMs())`. `Replica` only needs to forward both values to `LogTablet` and optionally log the old/new effective value. There is no need to delegate updates to `LogManager` or scan `currentLogs`.
This keeps the update complexity at `O(number of tables + number of replicas)`, gives `LogTablet` sole ownership of the runtime policy it executes, and ensures that creation, recovery, SET, and RESET all derive `effectiveLocalLogTtlMs` through the same implementation.
--
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] Support independent local log TTL [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on code in PR #3778:
URL: https://github.com/apache/fluss/pull/3778#discussion_r3772293615
##########
fluss-client/src/test/java/org/apache/fluss/client/admin/FlussAdminITCase.java:
##########
@@ -928,6 +951,24 @@ void testCreateTableWithInvalidProperty() {
"Currently, Primary Key Table supports ARROW or COMPACTED log format when kv format is COMPACTED.");
}
+ @Test
+ void testCreateTableWithLocalTtlAndInfiniteRemoteTtl() throws Exception {
+ TablePath tablePath =
+ TablePath.of(
+ DEFAULT_TABLE_PATH.getDatabaseName(),
+ "test_local_ttl_with_infinite_remote_ttl");
+ TableDescriptor tableDescriptor =
+ TableDescriptor.builder()
+ .schema(DEFAULT_SCHEMA)
+ .property(ConfigOptions.TABLE_LOG_TTL.key(), "0ms")
Review Comment:
If `0ms` is not allowed for `TABLE_LOG_LOCaAL_TTL`, why is it permitted for `TABLE_LOG_TTL`? I think we should allow `0` to disable TTL, just like `table.log.ttl` and align with `table.log.ttl`.
##########
website/docs/maintenance/tiered-storage/remote-storage.md:
##########
@@ -35,7 +35,10 @@ Below is the list for all configurations to control the log segments tiered beha
When local log segments are copied to remote storage, the local log segments will be deleted to reduce local disk cost.
But sometimes, we want to keep the several latest log segments retain in local, although they have been coped to remote storage for better read performance.
-You can control how many log segments to retain in local by setting the configuration `table.log.tiered.local-segments`(default is 2) per table.
+You can control local retention per table with `table.log.tiered.local-segments` (default is 2)
+and `table.log.local-ttl` (default is 1 day). A configured local TTL must be greater than zero
+and, when `table.log.ttl` is positive, less than or equal to `table.log.ttl`. A local segment is
+deleted only after it has been copied to remote storage.
Review Comment:
"A local segment" --> "An expired local segment"
##########
fluss-server/src/test/java/org/apache/fluss/server/log/remote/TieredLocalSegmentTTLTest.java:
##########
@@ -94,34 +100,24 @@ void testInactiveTieredLocalSegmentRemovedAfterTtl(boolean partitionTable) throw
@ParameterizedTest
@ValueSource(booleans = {true, false})
- void testExpiredActiveSegmentWaitsForHighWatermarkWithoutRemoteLogEndOffset(
Review Comment:
Why this test is removed? I think this is meaningful to test expire active segments.
--
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] Support independent local log TTL [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on code in PR #3778:
URL: https://github.com/apache/fluss/pull/3778#discussion_r3774697905
##########
fluss-common/src/main/java/org/apache/fluss/config/FlussConfigUtils.java:
##########
@@ -52,6 +52,7 @@ public class FlussConfigUtils {
ConfigOptions.TABLE_DATALAKE_FRESHNESS.key(),
ConfigOptions.TABLE_DATALAKE_AUTO_COMPACTION.key(),
ConfigOptions.TABLE_TIERED_LOG_LOCAL_SEGMENTS.key(),
+ ConfigOptions.TABLE_LOG_LOCAL_TTL.key(),
ConfigOptions.TABLE_AUTO_PARTITION_ENABLED.key(),
Review Comment:
`table.log.ttl` is the more commonly used retention option and remains authoritative when remote tiering is disabled, as well as the fallback when `table.log.local-ttl` is unset. This PR only makes `table.log.local-ttl` hot-updatable, so let's keep the current scope and add hot-update support for setting and resetting `table.log.ttl`, including propagation to online replicas, in the follow-up tracked by [#3231](https://github.com/apache/fluss/issues/3231).
--
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] Support independent local log TTL [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on code in PR #3778:
URL: https://github.com/apache/fluss/pull/3778#discussion_r3773330669
##########
fluss-server/src/test/java/org/apache/fluss/server/log/remote/TieredLocalSegmentTTLTest.java:
##########
@@ -94,34 +100,24 @@ void testInactiveTieredLocalSegmentRemovedAfterTtl(boolean partitionTable) throw
@ParameterizedTest
@ValueSource(booleans = {true, false})
- void testExpiredActiveSegmentWaitsForHighWatermarkWithoutRemoteLogEndOffset(
- boolean partitionTable) throws Exception {
+ void testExpiredLocalSegmentsRetainedWithoutHighestCopiedEndOffset(boolean partitionTable)
+ throws Exception {
TableBucket tb =
partitionTable
? new TableBucket(DATA1_TABLE_ID, 0L, 0)
: new TableBucket(DATA1_TABLE_ID, 0);
- conf.set(ConfigOptions.LOG_RETENTION_ROLL_ACTIVE_SEGMENT_ENABLED, true);
- logManager.reconfigure(conf);
makeLogTableAsLeader(tb, partitionTable);
LogTablet logTablet = replicaManager.getReplicaOrException(tb).getLogTablet();
addMultiSegmentsToLogTablet(logTablet, 5);
assertThat(remoteLogManager.remoteLogTablet(tb).getRemoteLogEndOffset()).isEmpty();
- manualClock.advanceTime(Duration.ofHours(2));
- logTablet.updateHighWatermark(logTablet.localLogEndOffset() - 1L);
+ manualClock.advanceTime(Duration.ofMinutes(90));
Review Comment:
We should advance the time for > 2 hours, to validate the log can be expired by the high-watermark.
##########
fluss-common/src/main/java/org/apache/fluss/config/ConfigOptions.java:
##########
@@ -1835,6 +1835,16 @@ public class ConfigOptions {
"The number of log segments to retain in local for each table when log tiered storage is enabled. "
+ "It must be greater that 0. The default is 2.");
+ public static final ConfigOption<Duration> TABLE_LOG_LOCAL_TTL =
+ key("table.log.local-ttl")
+ .durationType()
+ .defaultValue(Duration.ofDays(1))
+ .withDescription(
+ "The time to retain local log segments after they have been copied to remote storage. "
Review Comment:
Improved a bit for the descriptions
```
public static final ConfigOption<Duration> TABLE_LOG_TTL =
key("table.log.ttl")
.durationType()
.defaultValue(Duration.ofDays(7))
.withDescription(
"The time to live for log segments. The configuration controls the "
+ "maximum time log segments are retained before they become "
+ "eligible for deletion. When remote log tiering is enabled, "
+ "this value controls the retention of remote log segments. "
+ "Setting the value to -1 disables TTL-based deletion. "
+ "The default value is 7 days.");
public static final ConfigOption<Duration> TABLE_LOG_LOCAL_TTL =
key("table.log.local-ttl")
.durationType()
.noDefaultValue()
.withDescription(
"The time to live for local log segments. The configuration controls the "
+ "maximum time local log segments are retained before they become "
+ "eligible for deletion. When remote log tiering is enabled, an "
+ "expired local segment is deleted only after it has been copied "
+ "to remote storage. Setting the value to -1 disables TTL-based "
+ "deletion. If not configured, the value inherits `table.log.ttl`. "
+ "When both values are positive, it must be less than or equal to "
+ "`table.log.ttl`.");
```
--
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] Support independent local log TTL [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on code in PR #3778:
URL: https://github.com/apache/fluss/pull/3778#discussion_r3773123268
##########
fluss-common/src/main/java/org/apache/fluss/config/ConfigOptions.java:
##########
@@ -1835,6 +1835,16 @@ public class ConfigOptions {
"The number of log segments to retain in local for each table when log tiered storage is enabled. "
+ "It must be greater that 0. The default is 2.");
+ public static final ConfigOption<Duration> TABLE_LOG_LOCAL_TTL =
+ key("table.log.local-ttl")
+ .durationType()
+ .defaultValue(Duration.ofDays(1))
Review Comment:
Please leave `table.log.local-ttl` unset by default and fall back to `table.log.ttl`, as Kafka does with `local.retention.ms`. With a fixed one-day default, existing tables can silently change their local retention after an upgrade, causing abrupt changes in disk usage and read performance. A table configured only with `table.log.ttl=1h` also bypasses this invariant check because validation uses `getOptional(TABLE_LOG_LOCAL_TTL)`, while runtime cleanup still uses the one-day default, so its effective local TTL exceeds the remote TTL. This violates the `local TTL <= remote TTL` constraint and the backward-compatibility requirement in [Issue #3748](https://github.com/apache/fluss/issues/3748). Please keep the new option without a default and make `TableConfig#getLocalLogTTLMs()` use it only when explicitly configured; otherwise, return `table.log.ttl`. Resetting the option should restore the same fallback.
--
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] Support independent local log TTL [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong merged PR #3778:
URL: https://github.com/apache/fluss/pull/3778
--
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