You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "Zakelly (via GitHub)" <gi...@apache.org> on 2024/02/26 13:17:29 UTC
[PR] [DRAFT][FLINK-34516] Move CheckpointingMode to flink-core [flink]
Zakelly opened a new pull request, #24381:
URL: https://github.com/apache/flink/pull/24381
## Do not merge this for now
## What is the purpose of the change
In FLIP-406, we want to merge all options in `ExecutionCheckpointingOptions` to `CheckpointingOptions`, which depends on the `CheckpointingMode` being moved to `flink-core` as well. This PR introduce a new `CheckpointingMode` in `flink-core`, and deprecate the old one as well as corresponding user-facing APIs.
## Brief change log
- introduce a new `CheckpointingMode` in `flink-core`, providing new APIs.
- deprecate the old `CheckpointingMode` in `flink-streaming-java` as well as the APIs.
## Verifying this change
This change is already covered by new introduced UTs.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (**yes** / no)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (not applicable / docs / **JavaDocs** / not documented)
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1513823880
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -178,7 +178,9 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getCheckpointMode} instead.
*/
+ @Deprecated
public CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
Review Comment:
> IIUC, we should replace all internal usages of original CheckpointMode and docs with new one except for the user-facing interface, right?
Yeah, and this will be addressed by another PR.
> I'd suggest to transform to the new CheckpointMode for all original public interfaces, and use the new one for other inner places.
Well, I suggest we make this easier since the public API and the option will be removed at the same time, no more complex conversion needed, WDYT?
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1533247878
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -177,20 +177,44 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getConsistencyMode} instead.
*/
- public CheckpointingMode getCheckpointingMode() {
+ @Deprecated
+ public org.apache.flink.streaming.api.CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
}
/**
* Sets the checkpointing mode (exactly-once vs. at-least-once).
*
* @param checkpointingMode The checkpointing mode.
+ * @deprecated Use {@link #setConsistencyMode} instead.
*/
- public void setCheckpointingMode(CheckpointingMode checkpointingMode) {
+ @Deprecated
+ public void setCheckpointingMode(
+ org.apache.flink.streaming.api.CheckpointingMode checkpointingMode) {
configuration.set(ExecutionCheckpointingOptions.CHECKPOINTING_MODE, checkpointingMode);
}
+ /**
+ * Gets the checkpointing consistency mode (exactly-once vs. at-least-once).
+ *
+ * @return The checkpointing mode.
+ */
+ public CheckpointingMode getConsistencyMode() {
Review Comment:
This looks good to me.
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1529594692
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -177,20 +177,44 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getConsistencyMode} instead.
*/
- public CheckpointingMode getCheckpointingMode() {
+ @Deprecated
+ public org.apache.flink.streaming.api.CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
}
/**
* Sets the checkpointing mode (exactly-once vs. at-least-once).
*
* @param checkpointingMode The checkpointing mode.
+ * @deprecated Use {@link #setConsistencyMode} instead.
*/
- public void setCheckpointingMode(CheckpointingMode checkpointingMode) {
+ @Deprecated
+ public void setCheckpointingMode(
+ org.apache.flink.streaming.api.CheckpointingMode checkpointingMode) {
configuration.set(ExecutionCheckpointingOptions.CHECKPOINTING_MODE, checkpointingMode);
}
+ /**
+ * Gets the checkpointing consistency mode (exactly-once vs. at-least-once).
+ *
+ * @return The checkpointing mode.
+ */
+ public CheckpointingMode getConsistencyMode() {
Review Comment:
CheckpointingConsistencyMode looks good to me.
Could we also update the config name and the class name since it's also a public API same as config ?
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-2014287639
merged 8fac8046...77945916 into master
--
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
Re: [PR] [DRAFT][FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-1964137824
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "9bdd237d0322df8853f1b9e6ae658f77b9175237",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9bdd237d0322df8853f1b9e6ae658f77b9175237",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9bdd237d0322df8853f1b9e6ae658f77b9175237 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1533244572
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -177,20 +177,44 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getConsistencyMode} instead.
*/
- public CheckpointingMode getCheckpointingMode() {
+ @Deprecated
+ public org.apache.flink.streaming.api.CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
}
/**
* Sets the checkpointing mode (exactly-once vs. at-least-once).
*
* @param checkpointingMode The checkpointing mode.
+ * @deprecated Use {@link #setConsistencyMode} instead.
*/
- public void setCheckpointingMode(CheckpointingMode checkpointingMode) {
+ @Deprecated
+ public void setCheckpointingMode(
+ org.apache.flink.streaming.api.CheckpointingMode checkpointingMode) {
configuration.set(ExecutionCheckpointingOptions.CHECKPOINTING_MODE, checkpointingMode);
}
+ /**
+ * Gets the checkpointing consistency mode (exactly-once vs. at-least-once).
+ *
+ * @return The checkpointing mode.
+ */
+ public CheckpointingMode getConsistencyMode() {
Review Comment:
I'd change the API name `get/setConsistencyMode` to `get/setCheckpointingConsistencyMode` and add some description showing the relationship between the name `CheckpointingConsistencyMode` and `CheckpointingMode`. But I'd suggest keep `CheckpointingMode` as it is since it is a first class citizen of Flink and widely accepted by users. WDYT?
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx closed pull request #24381: [FLINK-34516] Move CheckpointingMode to flink-core
URL: https://github.com/apache/flink/pull/24381
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1513788931
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -178,7 +178,9 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getCheckpointMode} instead.
*/
+ @Deprecated
public CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
Review Comment:
IIUC, we should replace all internal usages of original CheckpointMode and docs with new one except for the user-facing interface, right?
I'd suggest to transform to the new CheckpointMode for all original public interfaces, and use the new one for other inner places.
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-1994372709
@flinkbot run azure
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-1996356653
@masteryhx I have transformed to the new CheckpointMode everywhere, and rebased master. Would you please take a look?
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-1999086935
@flinkbot run azure
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "masteryhx (via GitHub)" <gi...@apache.org>.
masteryhx commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1533238343
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -177,20 +177,44 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getConsistencyMode} instead.
*/
- public CheckpointingMode getCheckpointingMode() {
+ @Deprecated
+ public org.apache.flink.streaming.api.CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
}
/**
* Sets the checkpointing mode (exactly-once vs. at-least-once).
*
* @param checkpointingMode The checkpointing mode.
+ * @deprecated Use {@link #setConsistencyMode} instead.
*/
- public void setCheckpointingMode(CheckpointingMode checkpointingMode) {
+ @Deprecated
+ public void setCheckpointingMode(
+ org.apache.flink.streaming.api.CheckpointingMode checkpointingMode) {
configuration.set(ExecutionCheckpointingOptions.CHECKPOINTING_MODE, checkpointingMode);
}
+ /**
+ * Gets the checkpointing consistency mode (exactly-once vs. at-least-once).
+ *
+ * @return The checkpointing mode.
+ */
+ public CheckpointingMode getConsistencyMode() {
Review Comment:
I just have a minor concern that users may lost about `CheckpointingMode` and `ConsistencyMode`.
How about renaming this to `CheckpointingConsistencyMode` and adding some descriptions about the relationship between them ?
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on code in PR #24381:
URL: https://github.com/apache/flink/pull/24381#discussion_r1529761768
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/CheckpointConfig.java:
##########
@@ -177,20 +177,44 @@ public boolean isCheckpointingEnabled() {
* Gets the checkpointing mode (exactly-once vs. at-least-once).
*
* @return The checkpointing mode.
+ * @deprecated Use {@link #getConsistencyMode} instead.
*/
- public CheckpointingMode getCheckpointingMode() {
+ @Deprecated
+ public org.apache.flink.streaming.api.CheckpointingMode getCheckpointingMode() {
return configuration.get(ExecutionCheckpointingOptions.CHECKPOINTING_MODE);
}
/**
* Sets the checkpointing mode (exactly-once vs. at-least-once).
*
* @param checkpointingMode The checkpointing mode.
+ * @deprecated Use {@link #setConsistencyMode} instead.
*/
- public void setCheckpointingMode(CheckpointingMode checkpointingMode) {
+ @Deprecated
+ public void setCheckpointingMode(
+ org.apache.flink.streaming.api.CheckpointingMode checkpointingMode) {
configuration.set(ExecutionCheckpointingOptions.CHECKPOINTING_MODE, checkpointingMode);
}
+ /**
+ * Gets the checkpointing consistency mode (exactly-once vs. at-least-once).
+ *
+ * @return The checkpointing mode.
+ */
+ public CheckpointingMode getConsistencyMode() {
Review Comment:
@masteryhx Personally, I'd be fine with name the enum `CheckpointingConsistencyMode`, but that's a breaking change and needs more discussion. My current plan is to keep the `CheckpointingMode` here, and provide API with 'Consistency' in its name just for avoiding conflicts. We will reuse the original API name later in 2.0, WDYT?
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-1997034239
Rebased for conflicts....
--
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
Re: [PR] [FLINK-34516] Move CheckpointingMode to flink-core [flink]
Posted by "Zakelly (via GitHub)" <gi...@apache.org>.
Zakelly commented on PR #24381:
URL: https://github.com/apache/flink/pull/24381#issuecomment-1999431941
@flinkbot run azure
--
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