You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/06 07:37:29 UTC

[GitHub] [flink-table-store] tsreaper opened a new pull request, #75: [FLINK-27085] Introduce snapshot.num-retained.min option

tsreaper opened a new pull request, #75:
URL: https://github.com/apache/flink-table-store/pull/75

   Currently we retain at least 1 snapshot when expiring. However consider the following scenario:
   
   A user writes a snapshot one day ago. Today he is reading this snapshot and meanwhile writing more records. If a new snapshot is created and the reading is not finished, the old snapshot created one day ago will be removed as it exceeds maximum retaining time. This will cause the reading to fail.
   
   We should introduce snapshot.num-retained.min to at least retain a minimum number of snapshots to avoid this problem.


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-table-store] JingsongLi merged pull request #75: [FLINK-27085] Introduce snapshot.num-retained.min option

Posted by GitBox <gi...@apache.org>.
JingsongLi merged PR #75:
URL: https://github.com/apache/flink-table-store/pull/75


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-table-store] JingsongLi commented on a diff in pull request #75: [FLINK-27085] Introduce snapshot.num-retained.min option

Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #75:
URL: https://github.com/apache/flink-table-store/pull/75#discussion_r843625066


##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/FileStoreOptions.java:
##########
@@ -87,8 +87,14 @@
                             "The default partition name in case the dynamic partition"
                                     + " column value is null/empty string.");
 
-    public static final ConfigOption<Integer> SNAPSHOT_NUM_RETAINED =
-            ConfigOptions.key("snapshot.num-retained")
+    public static final ConfigOption<Integer> SNAPSHOT_NUM_RETAINED_MIN =
+            ConfigOptions.key("snapshot.num-retained.min")
+                    .intType()
+                    .defaultValue(10)
+                    .withDescription("The minimum number of completed snapshots to retain.");
+
+    public static final ConfigOption<Integer> SNAPSHOT_NUM_RETAINED_MAX =
+            ConfigOptions.key("snapshot.num-retained.max")

Review Comment:
   Please also modify ducument.



##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/operation/FileStoreExpireImpl.java:
##########
@@ -102,8 +105,8 @@ public void expire() {
         }
 
         // find the earliest snapshot to retain
-        for (long id = Math.max(latestSnapshotId - numRetained + 1, earliest);
-                id <= latestSnapshotId;

Review Comment:
   It seems `id < latestSnapshotId` is better in the past



-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org