rkhachatryan commented on a change in pull request #14754:
URL: https://github.com/apache/flink/pull/14754#discussion_r569721814
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/OperatorState.java
##########
@@ -81,10 +93,22 @@ public OperatorID getOperatorID() {
return operatorID;
}
+ public boolean isFullyFinished() {
+ return operatorSubtaskStates.equals(FULLY_FINISHED_STATE);
+ }
+
+ public void markedFullyFinished() {
+ operatorSubtaskStates.clear();
+ FULLY_FINISHED_STATE.forEach(operatorSubtaskStates::put);
+ }
+
public void putState(int subtaskIndex, OperatorSubtaskState subtaskState) {
Preconditions.checkNotNull(subtaskState);
+ checkState(!isFullyFinished());
- if (subtaskIndex < 0 || subtaskIndex >= parallelism) {
+ if (subtaskIndex == FULLY_FINISHED_PLACEHOLDER_INDEX) {
+ markedFullyFinished();
Review comment:
Just to clarify, this branch is for deserialization, right?
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -308,6 +316,25 @@ public CompletedCheckpoint finalizeCheckpoint(
// make sure we fulfill the promise with an exception if something
fails
try {
+ // Completes the operator state for the fully finished
operators
+ for (ExecutionJobVertex jobVertex :
checkpointBrief.getFullyFinishedJobVertex()) {
+ for (OperatorIDPair operatorID :
jobVertex.getOperatorIDs()) {
+ OperatorState operatorState =
+
operatorStates.get(operatorID.getGeneratedOperatorID());
+
+ if (operatorState == null) {
+ operatorState =
+ new OperatorState(
+
operatorID.getGeneratedOperatorID(),
+ jobVertex.getParallelism(),
+ jobVertex.getMaxParallelism());
+
operatorStates.put(operatorID.getGeneratedOperatorID(), operatorState);
+ }
+
+ operatorState.markedFullyFinished();
Review comment:
IIUC, the actual state should not be present for finished operators for
this checkpoint, right?
Can we add a check for this?
Otherwise, we need to discard previous states.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/OperatorState.java
##########
@@ -41,6 +41,18 @@
*/
public class OperatorState implements CompositeStateHandle {
+ private static final int FULLY_FINISHED_PLACEHOLDER_INDEX = -1;
+
+ /** A special content that marks this operator as fully finished. */
+ private static final Map<Integer, OperatorSubtaskState>
FULLY_FINISHED_STATE =
+ new HashMap<Integer, OperatorSubtaskState>() {
+ {
+ this.put(
+ FULLY_FINISHED_PLACEHOLDER_INDEX,
+ OperatorSubtaskState.builder().build());
+ }
+ };
Review comment:
Why not `Collections.singletonMap(FULLY_FINISHED_PLACEHOLDER_INDEX,
OperatorSubtaskState.builder().build()));`?
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/PendingCheckpoint.java
##########
@@ -308,6 +316,25 @@ public CompletedCheckpoint finalizeCheckpoint(
// make sure we fulfill the promise with an exception if something
fails
try {
+ // Completes the operator state for the fully finished
operators
+ for (ExecutionJobVertex jobVertex :
checkpointBrief.getFullyFinishedJobVertex()) {
Review comment:
nit: extract method?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
