Myasuka commented on code in PR #23509:
URL: https://github.com/apache/flink/pull/23509#discussion_r1423537299
##########
flink-runtime/src/main/java/org/apache/flink/runtime/state/memory/MemoryBackendCheckpointStorageAccess.java:
##########
@@ -72,6 +73,7 @@ public MemoryBackendCheckpointStorageAccess(
JobID jobId,
@Nullable Path checkpointsBaseDirectory,
@Nullable Path defaultSavepointLocation,
+ boolean createCheckpointSubDirs,
Review Comment:
How about introducing another constructor to make `createCheckpointSubDirs`
as default `true`? If so, we can avoid touching many test code in this PR.
##########
flink-runtime/src/main/java/org/apache/flink/runtime/state/filesystem/FsStateBackend.java:
##########
@@ -141,6 +141,12 @@ public class FsStateBackend extends
AbstractFileStateBackend implements Configur
*/
private final int writeBufferSize;
+ /**
+ * Switch to create checkpoint sub-directory with name of jobId. A value
of 'undefined' means
+ * not yet configured, in which case the default will be used.
+ */
+ private TernaryBoolean createCheckpointSubDirs = TernaryBoolean.UNDEFINED;
Review Comment:
I wonder do we really need to introduce an undefined Boolean here in such
way.
Currently, we only have one way to set this value within a private
constructor:
~~~java
this.createCheckpointSubDirs =
original.createCheckpointSubDirs.resolveUndefined(
configuration.get(CheckpointingOptions.CREATE_CHECKPOINT_SUB_DIS));
~~~
However, there is no public constructor or setter to set the
`createCheckpointSubDirs` in the `original` one, that is to say we will always
get a `TernaryBoolean.UNDEFINED` `createCheckpointSubDirs` in the original.
--
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-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
