Savonitar commented on code in PR #28709:
URL: https://github.com/apache/flink/pull/28709#discussion_r3591070584
##########
flink-libraries/flink-state-processing-api/src/main/java/org/apache/flink/state/api/output/SavepointOutputFormat.java:
##########
@@ -79,7 +79,19 @@ public void writeRecord(CheckpointMetadata metadata) throws
IOException {
() -> {
try (CheckpointMetadataOutputStream out =
targetLocation.createMetadataOutputStream()) {
- Checkpoints.storeCheckpointMetadata(metadata,
out);
+ // Must stay on the variant WITHOUT the
exclusive directory. Files
+ // retained from an existing savepoint are
copied into this
+ // directory by FileCopyFunction, but their
handles still reference
+ // the source savepoint. Writing without the
exclusive directory
+ // keeps the relative (file-name-only)
encoding, which resolves
+ // against this directory on restore and keeps
the savepoint
+ // self-contained. The exclusive-dir-aware
+ // Checkpoints.storeCheckpointMetadata would
instead fill this
+ // savepoint's metadata with absolute paths
pointing into the
+ // source savepoint, breaking this savepoint
once the source is
+ // deleted.
+
Checkpoints.storeCheckpointMetadataWithoutExclusiveDir(
+ metadata, out);
Review Comment:
> FileCopyFunction what's that?
FileCopyFunction is a state processor API thing.
> Who is copying what? SavepointOutputFormat doesn't have to be invovled in
copy any files AFAIU.
SavepointOutputFormat has one prod caller: SavepointWriter.write() the
method behind fromExistingSavepoint(...) and that method builds a batch job
with two sinks: One sink (FileCopyFunction) copies every state file referenced
by the carried-over operator states into the new savepoint directory, keeping
the file name. The other sink (this SavepointOutputFormat) writes the
_metadata. So you're right and this class copies nothing but the other sink of
the same job does.
> Again I'm confused who is copying what, and why are we talking about
source in this class/method.
The "source" is the savepoint from which one we build new one, the
carried-over handles still record paths inside it, nothing rewrites them. With
`storeCheckpointMetadataWithoutExclusiveDir` the metadata stores only file
names, which resolve to the local copies on restore, so the source can be
deleted.
I updated the comment with this context in
https://github.com/apache/flink/pull/28709/changes/6be50ec12534db4fe138f2b981907b6c39410248
--
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]
