zhijiangW commented on a change in pull request #11515: [FLINK-16744][task]
implement channel state persistence for unaligned checkpoints
URL: https://github.com/apache/flink/pull/11515#discussion_r403484216
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/state/TaskStateManagerImpl.java
##########
@@ -68,18 +70,36 @@
/** The checkpoint responder through which this manager can report to
the job manager. */
private final CheckpointResponder checkpointResponder;
+ private final ChannelStateReader channelStateReader;
+
public TaskStateManagerImpl(
- @Nonnull JobID jobId,
- @Nonnull ExecutionAttemptID executionAttemptID,
- @Nonnull TaskLocalStateStore localStateStore,
- @Nullable JobManagerTaskRestore jobManagerTaskRestore,
- @Nonnull CheckpointResponder checkpointResponder) {
+ @Nonnull JobID jobId,
+ @Nonnull ExecutionAttemptID executionAttemptID,
+ @Nonnull TaskLocalStateStore localStateStore,
+ @Nullable JobManagerTaskRestore jobManagerTaskRestore,
+ @Nonnull CheckpointResponder checkpointResponder) {
+ this(jobId,
+ executionAttemptID,
+ localStateStore,
+ jobManagerTaskRestore,
+ checkpointResponder,
+ new ChannelStateReaderImpl(jobManagerTaskRestore ==
null ? new TaskStateSnapshot() : jobManagerTaskRestore.getTaskStateSnapshot())
+ );
+ }
- this.jobId = jobId;
+ TaskStateManagerImpl(
Review comment:
Yes, I agree with your point to some extent. I think the key concern is how
we define the access modifier based on two considerations.
- Based on current demands: if so, it should be private ATM, and then
further extend it by demands if necessary future.
- Based on future considerations: if so, it can be defined as package public
now, even public. But it is hard to say whether it is alway fitting the
expectation, then it might seem unnecessary for long time. If taking this
option, we might even remove `@VisibleForTesting` annotation for previous
usages, because any constructors might have the possibility to be used in core
codes future.
Anyway, I can accept both options and do not think it is a big issue. Just
share some thoughts, feel free to take for your favor. :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
