Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-20 Thread via GitHub
Zakelly merged PR #24806: URL: https://github.com/apache/flink/pull/24806 -- 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.apach

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-20 Thread via GitHub
Zakelly commented on PR #24806: URL: https://github.com/apache/flink/pull/24806#issuecomment-2120406788 @fredia Thanks for your comments! I've addressed them and PTAL. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use th

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-20 Thread via GitHub
Zakelly commented on code in PR #24806: URL: https://github.com/apache/flink/pull/24806#discussion_r1606346053 ## flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/filemerging/FileMergingSnapshotManagerBase.java: ## @@ -485,6 +507,36 @@ public void notifyCheckpoint

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-20 Thread via GitHub
Zakelly commented on code in PR #24806: URL: https://github.com/apache/flink/pull/24806#discussion_r1606342513 ## flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/filemerging/SubtaskFileMergingManagerRestoreOperation.java: ## @@ -93,8 +94,12 @@ public void restore

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-20 Thread via GitHub
Zakelly commented on PR #24806: URL: https://github.com/apache/flink/pull/24806#issuecomment-2119802854 > Thanks for the PR. I left two comments. > > > File manager cannot recognize the `PlaceholderStreamStateHandle` > > Theoretically, `PlaceholderStreamStateHandle` should not b

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-19 Thread via GitHub
fredia commented on code in PR #24806: URL: https://github.com/apache/flink/pull/24806#discussion_r1606322375 ## flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/filemerging/FileMergingSnapshotManagerBase.java: ## @@ -485,6 +507,36 @@ public void notifyCheckpointS

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-19 Thread via GitHub
Zakelly commented on PR #24806: URL: https://github.com/apache/flink/pull/24806#issuecomment-2119753900 @flinkbot run azure -- 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.

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-17 Thread via GitHub
Zakelly commented on PR #24806: URL: https://github.com/apache/flink/pull/24806#issuecomment-2117278200 @flinkbot run azure -- 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.

Re: [PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-17 Thread via GitHub
flinkbot commented on PR #24806: URL: https://github.com/apache/flink/pull/24806#issuecomment-2116975542 ## CI report: * 32645c6c59a013c3a49d4665fc23b1322caa4e9f UNKNOWN Bot commands The @flinkbot bot supports the following commands: - `@flinkbot run azure`

[PR] [FLINK-35379][Checkpoint] Fix incorrect checkpoint notification handling in file merging [flink]

2024-05-17 Thread via GitHub
Zakelly opened a new pull request, #24806: URL: https://github.com/apache/flink/pull/24806 ## What is the purpose of the change This PR fix several issues with file merging mechanism, see `Brief change log`. ## Brief change log Fix following issues: - Checkpoint noti