pnowojski commented on a change in pull request #17774:
URL: https://github.com/apache/flink/pull/17774#discussion_r767881154
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/DeactivatedCheckpointCompletedCheckpointStore.java
##########
@@ -69,6 +69,11 @@ public SharedStateRegistry getSharedStateRegistry() {
throw unsupportedOperationException();
Review comment:
For the sake of consistency and safety I think it would be better to
have it throwing an exception. If this class shouldn't be used (and that's the
intention), then better make sure it won't be used at least on the runtime
level. Also it seems it should be working fine with throwing version (based on
the usages). Or at least not worse than it used to.
For the record, I don't like the throwing version. Ideally with
checkpointing disabled, this class should just not be instantiated (for example
even having field `SchedulerBase#completedCheckpointStore` as `Optional` or
`@Nulalble` would be better), but that's out of scope of this ticket.
--
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]
