Savonitar commented on code in PR #28709:
URL: https://github.com/apache/flink/pull/28709#discussion_r3645111379
##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/metadata/MetadataSerializer.java:
##########
@@ -46,9 +49,42 @@ CheckpointMetadata deserialize(
throws IOException;
/**
- * Serializes a savepoint or checkpoint metadata to an output stream.
+ * Serializes a savepoint or checkpoint metadata to an output stream
without knowledge of the
+ * exclusive directory the metadata is written into.
+ *
+ * <p>This is a convenience variant of {@link
#serialize(CheckpointMetadata, DataOutputStream,
+ * Path)} with a {@code null} exclusive directory: relative file
references keep the relative
+ * encoding unconditionally and are resolved against whatever directory
the metadata is later
+ * read from, which is only correct if every referenced file actually
lives in that directory.
Review Comment:
> Shouldn't we instead use ../../claimed-files/chk-1/foo/1.sst? 🤔
A few reasons for the absolute path instead of the suggested "../../":
1. Same behavior as incremental checkpoints. Shared state (shared/) is
referenced by absolute path, which is also why incremental checkpoints can't be
relocated. A claimed savepoint SST reused by the first incremental checkpoint
is the same type of "lives outside the exclusive dir" reference, so if we keep
it absolute it will be consistent with the existing model instead of adding
new.
2. state.savepoints.dir and state.checkpoints.dir are independently
configurable and can live on different filesystems/buckets/schemes.
--
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]
