Myasuka commented on code in PR #19331:
URL: https://github.com/apache/flink/pull/19331#discussion_r844070832
##########
flink-runtime/src/main/java/org/apache/flink/runtime/state/SharedStateRegistry.java:
##########
@@ -66,10 +67,25 @@ StreamStateHandle registerReference(
/**
* Register given shared states in the registry.
*
+ * <p>NOTE: For state from checkpoints from other jobs or runs (i.e. after
recovery), please use
+ * {@link #registerAllAfterRestored(CompletedCheckpoint, RestoreMode)}
+ *
* @param stateHandles The shared states to register.
* @param checkpointID which uses the states.
*/
void registerAll(Iterable<? extends CompositeStateHandle> stateHandles,
long checkpointID);
+ /**
+ * Set the lowest checkpoint ID below which no state is discarded,
inclusive.
+ *
+ * <p>After recovery from an incremental checkpoint, its state should NOT
be discarded, even if
+ * {@link #unregisterUnusedState(long) not used} anymore (unless
recovering in {@link
+ * RestoreMode#CLAIM CLAIM} mode).
+ *
+ * <p>This should hold for both cases: when recovering from that initial
checkpoint; and from
+ * any subsequent checkpoint derived from it.
+ */
+ void registerAllAfterRestored(CompletedCheckpoint checkpoint, RestoreMode
mode);
Review Comment:
I think current dilemma is caused by that we might not make the refactory of
equivalence.
Previously, no matter whether JM crashed or just restore from
checkpoint/savepoint. All checkpoints would be registered to incrase the
reference counting (via
`CompletedCheckpoint#registerSharedStatesAfterRestored`), and the checkpoint
property of `discardOnSubume` could help whether to discard this registered
checkpoint to decrease the reference on subsume.
However, this changed after FLINK-24611. Previously, the unregister phase
for SharedStateReigstry does not depend on the checkpoint id relationship, we
just unregister the checkpoints independently and not consider which checkpoint
id is larger. However, after FLINK-24611, this changed and I think we could
persist the restored id in some place to mitigate such regression.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
