You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "LiebingYu (via GitHub)" <gi...@apache.org> on 2025/11/06 07:12:10 UTC
[PR] [server] Prevent remote log TTL expiration until tiering completes [fluss]
LiebingYu opened a new pull request, #1940:
URL: https://github.com/apache/fluss/pull/1940
<!--
*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]). 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.
**(The sections below can be removed for hotfixes or typos)**
-->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #1919
<!-- 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] Prevent remote log TTL expiration until tiering completes [fluss]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #1940:
URL: https://github.com/apache/fluss/pull/1940#issuecomment-3742622396
@LiebingYu The latter approach (ignoring tiering progress after disabling) might lead to silent data loss. For users, this is often an "unexpected behavior." If data correctness issues occur because the data was deleted before being successfully tiered, it becomes extremely difficult for us to troubleshoot and recover. We should require an explicit "force" flag or a specific confirmation if they truly want to bypass the tiering safety check.
I still prefer to not to do ttl when user disable the data lake. Realistically, if a user has already enabled the data lake, the scenarios where they would want to permanently disable it may be rare(I'm not sure the real case).
--
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] Prevent remote log TTL expiration until tiering completes [fluss]
Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong merged PR #1940:
URL: https://github.com/apache/fluss/pull/1940
--
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] Prevent remote log TTL expiration until tiering completes [fluss]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1940:
URL: https://github.com/apache/fluss/pull/1940#discussion_r2686082603
##########
fluss-server/src/test/java/org/apache/fluss/server/log/remote/RemoteLogTTLTest.java:
##########
@@ -65,9 +67,19 @@ void testRemoteLogTTL(boolean partitionTable) throws Exception {
assertThat(remoteLog.getRemoteLogStartOffset()).isEqualTo(0L);
// manually trigger again to delete the expired log segment to remote and commit snapshot.
- // default 7 days TTL, this should expire all remote segments.
+ // since data lake is enabled and no data has been tired to data lake,
Review Comment:
Spelling error in comment: "tired" should be "tiered".
```suggestion
// since data lake is enabled and no data has been tiered to data lake,
```
##########
fluss-server/src/main/java/org/apache/fluss/server/log/remote/RemoteLogTablet.java:
##########
@@ -161,7 +174,15 @@ public List<RemoteLogSegment> expiredRemoteLogSegments(long currentTimeMs) {
long ts = entry.getKey();
if (currentTimeMs - ts > ttlMs) {
for (UUID uuid : entry.getValue()) {
- expiredSegments.add(idToRemoteLogSegment.get(uuid));
+ RemoteLogSegment segment = idToRemoteLogSegment.get(uuid);
+ if (isDataLakeEnable) {
Review Comment:
Variable name has spelling error: "isDataLakeEnable" should be "isDataLakeEnabled" for consistency with the field name and getter method.
##########
fluss-server/src/main/java/org/apache/fluss/server/log/remote/RemoteLogTablet.java:
##########
@@ -147,8 +147,21 @@ public List<RemoteLogSegment> allRemoteLogSegments() {
return inReadLock(lock, () -> currentManifest.getRemoteLogSegmentList());
}
- /** Returns the expired segments based on the given time. */
- public List<RemoteLogSegment> expiredRemoteLogSegments(long currentTimeMs) {
+ /**
+ * Returns the expired segments based on the given time and lake log end offset.
+ *
+ * <p>Only segments that have been tiered to lake (i.e., remoteLogEndOffset <= lakeLogEndOffset)
+ * can be safely deleted. This ensures that we don't delete segments that haven't been tiered to
+ * lake yet.
+ *
+ * @param currentTimeMs the current time in milliseconds
+ * @param isDataLakeEnable whether data lake is enabled
+ * @param lakeLogEndOffset the log end offset that has been synced to lake, -1 if no lake sync
+ * has occurred
+ * @return list of expired segments that can be safely deleted
+ */
+ public List<RemoteLogSegment> expiredRemoteLogSegments(
+ long currentTimeMs, boolean isDataLakeEnable, long lakeLogEndOffset) {
Review Comment:
Parameter name has spelling error: "isDataLakeEnable" should be "isDataLakeEnabled" for consistency with the field name "isDataLakeEnabled" and getter method "isDataLakeEnabled()". This affects both the parameter name and the javadoc.
--
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] Prevent remote log TTL expiration until tiering completes [fluss]
Posted by "LiebingYu (via GitHub)" <gi...@apache.org>.
LiebingYu commented on code in PR #1940:
URL: https://github.com/apache/fluss/pull/1940#discussion_r2692693830
##########
fluss-server/src/main/java/org/apache/fluss/server/log/LogTablet.java:
##########
@@ -516,9 +522,15 @@ public void updateMinRetainOffset(long minRetainOffset) {
}
}
+ public void updateIsDataLakeEnabled(boolean isDataLakeEnabled) {
+ this.isDataLakeEnabled = isDataLakeEnabled;
+ }
+
public void updateLakeTableSnapshotId(long snapshotId) {
if (snapshotId > this.lakeTableSnapshotId) {
this.lakeTableSnapshotId = snapshotId;
+ // it means the data lake is enabled if we have got the snapshot id
+ this.isDataLakeEnabled = true;
Review Comment:
Yes, we will handle this in https://github.com/apache/fluss/issues/2375.
--
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] Prevent remote log TTL expiration until tiering completes [fluss]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on code in PR #1940:
URL: https://github.com/apache/fluss/pull/1940#discussion_r2690317476
##########
fluss-server/src/main/java/org/apache/fluss/server/log/LogTablet.java:
##########
@@ -516,9 +522,15 @@ public void updateMinRetainOffset(long minRetainOffset) {
}
}
+ public void updateIsDataLakeEnabled(boolean isDataLakeEnabled) {
+ this.isDataLakeEnabled = isDataLakeEnabled;
+ }
+
public void updateLakeTableSnapshotId(long snapshotId) {
if (snapshotId > this.lakeTableSnapshotId) {
this.lakeTableSnapshotId = snapshotId;
+ // it means the data lake is enabled if we have got the snapshot id
+ this.isDataLakeEnabled = true;
Review Comment:
Currently, we rely this to update `isDataLakeEnabled` dynamically, in the other pr, we should rely on `updateMetataRequest` .
##########
fluss-server/src/main/java/org/apache/fluss/server/log/remote/RemoteLogTablet.java:
##########
@@ -147,8 +147,21 @@ public List<RemoteLogSegment> allRemoteLogSegments() {
return inReadLock(lock, () -> currentManifest.getRemoteLogSegmentList());
}
- /** Returns the expired segments based on the given time. */
- public List<RemoteLogSegment> expiredRemoteLogSegments(long currentTimeMs) {
+ /**
+ * Returns the expired segments based on the given time and lake log end offset.
+ *
+ * <p>Only segments that have been tiered to lake (i.e., remoteLogEndOffset <= lakeLogEndOffset)
+ * can be safely deleted. This ensures that we don't delete segments that haven't been tiered to
+ * lake yet.
+ *
+ * @param currentTimeMs the current time in milliseconds
+ * @param isDataLakeEnabled whether data lake is enabled
+ * @param lakeLogEndOffset the log end offset that has been synced to lake, -1 if no lake sync
+ * has occurred
+ * @return list of expired segments that can be safely deleted
+ */
+ public List<RemoteLogSegment> expiredRemoteLogSegments(
+ long currentTimeMs, boolean isDataLakeEnabled, long lakeLogEndOffset) {
Review Comment:
```suggestion
long currentTimeMs, Long lakeLogEndOffset) {
```
null represent lake table?
--
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] Prevent remote log TTL expiration until tiering completes [fluss]
Posted by "LiebingYu (via GitHub)" <gi...@apache.org>.
LiebingYu commented on PR #1940:
URL: https://github.com/apache/fluss/pull/1940#issuecomment-3495520551
There's another problem for remote log TTL expiration when data lake enable. Let's say:
1. User create a Fluss table with data lake enable.
2. User disable the data lake.
The problem in step 2 is: We don't notify the Replica when disable the data lake. So remote log for this table will never been expired if user don't enable data lake again.
I suggest:
1. Temporarily prohibit users from disabling the lake before we resolve 2.
2. We need to discuss how data TTL should be handled if the user disables the data lake. One option is to do nothing—in this case, if the user never re-enables the data lake, all historical data will remain in Fluss, which would obviously put significant pressure on Fluss’s storage. Another option is to notify the replica when the data lake is disabled, so that afterwards the data's TTL will no longer consider the progress of tiering to the lake; the user would need to accept the risk of losing lake data if they have set an aggressive TTL. I prefer the latter approach. What do you think?
--
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