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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
