You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "MonsterChenzhuo (via GitHub)" <gi...@apache.org> on 2023/09/26 06:49:31 UTC

[GitHub] [incubator-paimon] MonsterChenzhuo opened a new pull request, #1949: [WIP][flink] Auto create new tag for flink checkpoint

MonsterChenzhuo opened a new pull request, #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   <!-- Linking this pull request to the issue -->
   close #1942
   
   <img width="952" alt="图片" src="https://github.com/apache/incubator-paimon/assets/60029759/5e1b54e2-0854-4a28-9a3d-50ecc72c4977">
   The generation of tags is driven by checkpoints. Although no new data is written to trigger the creation of a new snapshot, tags can still be generated and point to the current latest snapshot.
   
   <!-- What is the purpose of the change -->
   
   ### 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@paimon.apache.org

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


Re: [PR] [flink] Auto create new tag for flink checkpoint [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi closed pull request #1949: [flink] Auto create new tag for flink checkpoint
URL: https://github.com/apache/incubator-paimon/pull/1949


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

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


[GitHub] [incubator-paimon] MonsterChenzhuo commented on pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#issuecomment-1733257269

   <img width="1011" alt="图片" src="https://github.com/apache/incubator-paimon/assets/60029759/c191722c-031e-4422-9681-5936cfb7cf4b">
   @JingsongLi  We conducted a 10-day online test, and the results met our expectations.


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

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


Re: [PR] [flink] Auto create new tag for flink checkpoint [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#issuecomment-1765624656

   When I developed https://github.com/apache/incubator-paimon/pull/2134
   I found that snapshots are our fundamental solution, and another way to solve this problem is to force the creation of snapshots for tables (in this configuration case, or add a new configuration option), which is a relatively simple thing.
   
   


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

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


[GitHub] [incubator-paimon] MonsterChenzhuo closed pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo closed pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint
URL: https://github.com/apache/incubator-paimon/pull/1949


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

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


[GitHub] [incubator-paimon] MonsterChenzhuo commented on a diff in pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#discussion_r1325937300


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/AutoTagForSavepointCommitterOperator.java:
##########
@@ -58,31 +60,27 @@ public class AutoTagForSavepointCommitterOperator<CommitT, GlobalCommitT>
         implements OneInputStreamOperator<CommitT, CommitT>,
                 SetupableStreamOperator,
                 BoundedOneInput {
-    public static final String SAVEPOINT_TAG_PREFIX = "savepoint-";
-
+    @VisibleForTesting public static final String SAVEPOINT_TAG_PREFIX = "savepoint-";
     private static final long serialVersionUID = 1L;
-

Review Comment:
   done.



##########
paimon-core/src/main/java/org/apache/paimon/utils/TagManager.java:
##########
@@ -61,7 +61,9 @@ public Path tagPath(String tagName) {
     /** Create a tag from given snapshot and save it in the storage. */
     public void createTag(Snapshot snapshot, String tagName) {
         checkArgument(!StringUtils.isBlank(tagName), "Tag name '%s' is blank.", tagName);
-        checkArgument(!tagExists(tagName), "Tag name '%s' already exists.", tagName);
+        if (tagExists(tagName)) {

Review Comment:
   done.



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

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


[GitHub] [incubator-paimon] MonsterChenzhuo commented on a diff in pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#discussion_r1325937651


##########
paimon-core/src/main/java/org/apache/paimon/tag/TagAutoCreation.java:
##########
@@ -115,10 +114,20 @@ private TagAutoCreation(
         }
     }
 
+    public void refreshTags() {

Review Comment:
   done.



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/AutoTagForSavepointCommitterOperator.java:
##########
@@ -94,7 +92,7 @@ public void initializeState(StreamTaskStateInitializer streamTaskStateManager)
         } finally {
             snapshotManager = snapshotManagerFactory.get();
             tagManager = tagManagerFactory.get();
-
+            tagAutoCreation = tagAutoCreationFactory.get();

Review Comment:
   done.



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

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


Re: [PR] [flink] Auto create new tag for flink checkpoint [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#issuecomment-1783001815

   #2188 should fixes this.


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

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


[GitHub] [incubator-paimon] MonsterChenzhuo closed pull request #1949: [flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo closed pull request #1949: [flink] Auto create new tag for flink checkpoint
URL: https://github.com/apache/incubator-paimon/pull/1949


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

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


[GitHub] [incubator-paimon] MonsterChenzhuo commented on pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#issuecomment-1727719279

   Colleagues reported that some problems were found during testing and may need to be modified tomorrow.


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

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


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1949: [flink] Auto create new tag for flink checkpoint

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#discussion_r1321272921


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/AutoTagForSavepointCommitterOperator.java:
##########
@@ -94,7 +92,7 @@ public void initializeState(StreamTaskStateInitializer streamTaskStateManager)
         } finally {
             snapshotManager = snapshotManagerFactory.get();
             tagManager = tagManagerFactory.get();
-
+            tagAutoCreation = tagAutoCreationFactory.get();

Review Comment:
   Maybe you can try to extract `tagAutoCreation` from `CommitterOperator`. It is better to maintain only one object for in the committer.



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

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


Re: [PR] [flink] Auto create new tag for flink checkpoint [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#issuecomment-1752620040

   I think we can use a generic way to implement this.
   For example, if the extractor is `ProcessTimeExtractor` in `TagAutoCreation`, we can launch a async thread to check tag creation.


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

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


[GitHub] [incubator-paimon] MonsterChenzhuo commented on pull request #1949: [flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#issuecomment-1723248639

   @JingsongLi  PATL,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@paimon.apache.org

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


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1949: [flink] Auto create new tag for flink checkpoint

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#discussion_r1318169900


##########
docs/content/maintenance/manage-tags.md:
##########
@@ -257,6 +257,11 @@ Paimon's snapshot is similar to flink's checkpoint, and both will automatically
 allows snapshots to be retained for a long time. Therefore, we can combine the two features of paimon's tag and flink's 
 savepoint to achieve incremental recovery of job from the specified savepoint.
 
+At the same time to open the change parameter will also promote the automatic creation of the tag, now although open the "tag.automatic-creation" parameter but,

Review Comment:
   remove this sentence.. there is no `sink.savepoint.auto-tag` here.
   
   Document after `You can set `sink.savepoint.auto-tag` to `true` to enable the feature of automatically creating tags for savepoint.`



##########
docs/content/maintenance/manage-tags.md:
##########
@@ -257,6 +257,11 @@ Paimon's snapshot is similar to flink's checkpoint, and both will automatically
 allows snapshots to be retained for a long time. Therefore, we can combine the two features of paimon's tag and flink's 
 savepoint to achieve incremental recovery of job from the specified savepoint.
 
+At the same time to open the change parameter will also promote the automatic creation of the tag, now although open the "tag.automatic-creation" parameter but,

Review Comment:
   Just simplify documentation. For example: `This option will also force creating a Tag for the tag.automatic-creation=process-time mode at an appropriate time even when no data is written.`



##########
paimon-core/src/main/java/org/apache/paimon/utils/TagManager.java:
##########
@@ -61,7 +61,9 @@ public Path tagPath(String tagName) {
     /** Create a tag from given snapshot and save it in the storage. */
     public void createTag(Snapshot snapshot, String tagName) {
         checkArgument(!StringUtils.isBlank(tagName), "Tag name '%s' is blank.", tagName);
-        checkArgument(!tagExists(tagName), "Tag name '%s' already exists.", tagName);
+        if (tagExists(tagName)) {

Review Comment:
   Why add this?



##########
paimon-core/src/main/java/org/apache/paimon/tag/TagAutoCreation.java:
##########
@@ -115,10 +114,20 @@ private TagAutoCreation(
         }
     }
 
+    public void refreshTags() {

Review Comment:
   We should not add new method for `TagAutoCreation`.
   We should just invoke `run`.
   
   We can create a method:
   ```
   private void tryToTag(Snapshot snapshot, LocalDateTime time) {
           if (nextTag == null
                   || isAfterOrEqual(time.minus(delay), periodHandler.nextTagTime(nextTag))) {
               LocalDateTime thisTag = periodHandler.normalizeToTagTime(time);
               String tagName = periodHandler.timeToTag(thisTag);
               tagManager.createTag(snapshot, tagName);
               nextTag = periodHandler.nextTagTime(thisTag);
   
               if (numRetainedMax != null) {
                   SortedMap<Snapshot, String> tags = tagManager.tags();
                   if (tags.size() > numRetainedMax) {
                       int toDelete = tags.size() - numRetainedMax;
                       int i = 0;
                       for (String tag : tags.values()) {
                           tagManager.deleteTag(tag, tagDeletion, snapshotManager);
                           i++;
                           if (i == toDelete) {
                               break;
                           }
                       }
                   }
               }
           }
       }
   ```
   
   And in `run`:
   ```
       public void run() {
           while (true) {
               if (snapshotManager.snapshotExists(nextSnapshot)) {
                   tryToTag(snapshotManager.snapshot(nextSnapshot));
                   nextSnapshot++;
               } else {
                   // avoid snapshot has been expired
                   Long earliest = snapshotManager.earliestSnapshotId();
                   if (earliest != null && earliest > nextSnapshot) {
                       nextSnapshot = earliest;
                   } else {
                       break;
                   }
               }
           }
   
           if (timeExtractor instanceof ProcessTimeExtractor) {
               tryToTag(snapshotManager.snapshot(nextSnapshot - 1), LocalDateTime.now());
           }
       }
   ```
   
   Something like above codes.



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/AutoTagForSavepointCommitterOperator.java:
##########
@@ -58,31 +60,27 @@ public class AutoTagForSavepointCommitterOperator<CommitT, GlobalCommitT>
         implements OneInputStreamOperator<CommitT, CommitT>,
                 SetupableStreamOperator,
                 BoundedOneInput {
-    public static final String SAVEPOINT_TAG_PREFIX = "savepoint-";
-
+    @VisibleForTesting public static final String SAVEPOINT_TAG_PREFIX = "savepoint-";
     private static final long serialVersionUID = 1L;
-

Review Comment:
   Don't remove this empty line, this is just personal code style



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

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


[GitHub] [incubator-paimon] MonsterChenzhuo commented on a diff in pull request #1949: [WIP][flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #1949:
URL: https://github.com/apache/incubator-paimon/pull/1949#discussion_r1325936923


##########
docs/content/maintenance/manage-tags.md:
##########
@@ -257,6 +257,11 @@ Paimon's snapshot is similar to flink's checkpoint, and both will automatically
 allows snapshots to be retained for a long time. Therefore, we can combine the two features of paimon's tag and flink's 
 savepoint to achieve incremental recovery of job from the specified savepoint.
 
+At the same time to open the change parameter will also promote the automatic creation of the tag, now although open the "tag.automatic-creation" parameter but,

Review Comment:
   done.



##########
docs/content/maintenance/manage-tags.md:
##########
@@ -257,6 +257,11 @@ Paimon's snapshot is similar to flink's checkpoint, and both will automatically
 allows snapshots to be retained for a long time. Therefore, we can combine the two features of paimon's tag and flink's 
 savepoint to achieve incremental recovery of job from the specified savepoint.
 
+At the same time to open the change parameter will also promote the automatic creation of the tag, now although open the "tag.automatic-creation" parameter but,

Review Comment:
   done.



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

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


[GitHub] [incubator-paimon] MonsterChenzhuo closed pull request #1949: [flink] Auto create new tag for flink checkpoint

Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo closed pull request #1949: [flink] Auto create new tag for flink checkpoint
URL: https://github.com/apache/incubator-paimon/pull/1949


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

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