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/11/05 10:07:29 UTC
[PR] [core] Fix auto process time create snapshot for tag [incubator-paimon]
MonsterChenzhuo opened a new pull request, #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
https://github.com/apache/incubator-paimon/pull/2188/files
I conducted relevant tests locally using MySQL, and here are the screenshots:
<img width="1280" alt="企业微信截图_9d6c3269-8582-46cf-930a-09bc4d0f7e96" src="https://github.com/apache/incubator-paimon/assets/60029759/4ff87fe4-991e-4f00-86ac-30042cb69572">
There is an issue that arises, for instance, when I set the tag creation to once a day and checkpoints to occur every 2 minutes. This results in a large number of snapshot and manifest files being generated. I believe there is room for optimization. Each time a snapshot is forcefully created, it should be checked whether it coincides with the next scheduled tag creation time. If it does, proceed with the snapshot creation; otherwise, skip it.
### 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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#issuecomment-1805017860
> BTW, this is a good idea! Thanks @MonsterChenzhuo
😊 PTAL,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
Re: [PR] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo closed pull request #2262: [core] Fix auto process time create snapshot for tag
URL: https://github.com/apache/incubator-paimon/pull/2262
--
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#discussion_r1387642955
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/TableCommitImpl.java:
##########
@@ -120,7 +120,9 @@ public TableCommitImpl(
}
public boolean forceCreatingSnapshot() {
- return tagAutoCreation != null && tagAutoCreation.forceCreatingSnapshot();
+ return tagAutoCreation != null
+ && tagAutoCreation.canProceedWithNextTag()
Review Comment:
forceCreatingSnapshot first and then canProceedWithNextTag?
Why not just merge these two functions?
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/TableCommitImpl.java:
##########
@@ -120,7 +120,9 @@ public TableCommitImpl(
}
public boolean forceCreatingSnapshot() {
- return tagAutoCreation != null && tagAutoCreation.forceCreatingSnapshot();
+ return tagAutoCreation != null
+ && tagAutoCreation.canProceedWithNextTag()
Review Comment:
Why not just merge these two functions?
--
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#discussion_r1382917179
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/CommitterOperator.java:
##########
@@ -109,6 +119,18 @@ public void initializeState(StateInitializationContext context) throws Exception
committer = committerFactory.create(commitUser, getMetricGroup());
committableStateManager.initializeState(context, committer);
+
+ Class<?> storeCommitterClass = StoreCommitter.class;
+ Class<?> tableCommitImplClass = TableCommitImpl.class;
+ Field commitField = storeCommitterClass.getDeclaredField(COMMIT_FIELD_NAME);
Review Comment:
We don't recommend using reflection to fetch class field,
--
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] [WIP][core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#discussion_r1386048364
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/CommitterOperator.java:
##########
@@ -109,6 +119,18 @@ public void initializeState(StateInitializationContext context) throws Exception
committer = committerFactory.create(commitUser, getMetricGroup());
committableStateManager.initializeState(context, committer);
+
+ Class<?> storeCommitterClass = StoreCommitter.class;
+ Class<?> tableCommitImplClass = TableCommitImpl.class;
+ Field commitField = storeCommitterClass.getDeclaredField(COMMIT_FIELD_NAME);
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo closed pull request #2262: [core] Fix auto process time create snapshot for tag
URL: https://github.com/apache/incubator-paimon/pull/2262
--
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#discussion_r1387642955
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/TableCommitImpl.java:
##########
@@ -120,7 +120,9 @@ public TableCommitImpl(
}
public boolean forceCreatingSnapshot() {
- return tagAutoCreation != null && tagAutoCreation.forceCreatingSnapshot();
+ return tagAutoCreation != null
+ && tagAutoCreation.canProceedWithNextTag()
Review Comment:
`forceCreatingSnapshot` first and then `canProceedWithNextTag`?
--
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#issuecomment-1803376916
BTW, this is good idea! Thanks @MonsterChenzhuo
--
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#discussion_r1387642955
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/TableCommitImpl.java:
##########
@@ -120,7 +120,9 @@ public TableCommitImpl(
}
public boolean forceCreatingSnapshot() {
- return tagAutoCreation != null && tagAutoCreation.forceCreatingSnapshot();
+ return tagAutoCreation != null
+ && tagAutoCreation.canProceedWithNextTag()
Review Comment:
`forceCreatingSnapshot` first and then `canProceedWithNextTag`?
Why just merge these two functions?
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/TableCommitImpl.java:
##########
@@ -120,7 +120,9 @@ public TableCommitImpl(
}
public boolean forceCreatingSnapshot() {
- return tagAutoCreation != null && tagAutoCreation.forceCreatingSnapshot();
+ return tagAutoCreation != null
+ && tagAutoCreation.canProceedWithNextTag()
Review Comment:
`forceCreatingSnapshot` first and then `canProceedWithNextTag`?
Why not just merge these two functions?
--
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "MonsterChenzhuo (via GitHub)" <gi...@apache.org>.
MonsterChenzhuo commented on code in PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262#discussion_r1388818944
##########
paimon-core/src/main/java/org/apache/paimon/table/sink/TableCommitImpl.java:
##########
@@ -120,7 +120,9 @@ public TableCommitImpl(
}
public boolean forceCreatingSnapshot() {
- return tagAutoCreation != null && tagAutoCreation.forceCreatingSnapshot();
+ return tagAutoCreation != null
+ && tagAutoCreation.canProceedWithNextTag()
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] [core] Fix auto process time create snapshot for tag [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2262:
URL: https://github.com/apache/incubator-paimon/pull/2262
--
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