Savonitar commented on code in PR #28709:
URL: https://github.com/apache/flink/pull/28709#discussion_r3586024410
##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/Checkpoints.java:
##########
@@ -76,29 +78,80 @@ public class Checkpoints {
// Writing out checkpoint metadata
// ------------------------------------------------------------------------
- public static void storeCheckpointMetadata(
+ /**
+ * Stores the checkpoint metadata without knowing the checkpoint's
exclusive directory: relative
+ * file references are always persisted as relative and are resolved
against whatever directory
+ * the metadata is later read from. When writing the metadata of an actual
checkpoint or
+ * savepoint, prefer {@link #storeCheckpointMetadata(CheckpointMetadata,
+ * CheckpointMetadataOutputStream)}.
+ *
+ * <p>The deliberately different method name keeps the context-free
variants out of the {@code
+ * storeCheckpointMetadata} overload set, so a caller cannot switch
between the two encodings by
+ * merely changing a variable's static type.
+ */
+ public static void storeCheckpointMetadataWithoutExclusiveDir(
CheckpointMetadata checkpointMetadata, OutputStream out) throws
IOException {
DataOutputStream dos = new DataOutputStream(out);
- storeCheckpointMetadata(checkpointMetadata, dos);
+ storeCheckpointMetadataWithoutExclusiveDir(checkpointMetadata, dos);
}
+ /**
+ * Stores the checkpoint metadata into the given metadata output stream,
passing the stream's
+ * exclusive checkpoint directory to the serializer so that relative file
references stay
+ * self-consistent on recovery.
+ */
Review Comment:
> what are exclusive directories?
Just to clarify, this PR uses pre-PR dictionary. E.g. in
https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/metadata/MetadataV2V3SerializerBase.java#L934
or in
https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/state/CheckpointedStateScope.java#L31
we already have this term.
That's why I "reused" it. **_I'm happy to change it,_** if you can come up
with the better option.
**The exclusive directory** is the per-checkpoint directory that holds
_metadata and the checkpoint's non-shared state files, e.g. .../chk-100/, as
opposed to the job-wide shared/ directory.
> difference between the two methods.
1. `storeCheckpointMetadataWithoutExclusiveDir(...)`: pre-PR behaviour,
every relative handle is written relative unconditionally. It is correct
behavior when the caller guarantees every referenced file is located next to
the metadata (or will be located). State processor API establishes exactly this
guarantee by physically copying the files
2. `storeCheckpointMetadata(...)`: the serializer knows which exclusive
directory it is writing into and checks every RelativeFileStateHandle against
it. A handle whose file really lives there keeps the relative encoding, a
handle pointing elsewhere (the reused savepoint SST from this bug) is written
with its absolute path, because the relative form would be attched to chk-.../
on read and point at a file that was never there. That is what "self-consistent
on recovery" meant: nothing in the written metadata resolves to a wrong
location.
I will work on these java docs.
--
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]
