1996fanrui commented on code in PR #25030:
URL: https://github.com/apache/flink/pull/25030#discussion_r1667861271
##########
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/filemerging/FileMergingSnapshotManagerBase.java:
##########
@@ -773,6 +773,13 @@ static boolean
shouldSyncAfterClosingLogicalFile(FileSystem fileSystem) {
return true;
}
+ private static String uriEscape(String input) {
+ // All reserved characters (RFC 2396) will be removed. This is enough
for flink's resource
+ // id, job id and operator id.
+ // Ref:
https://docs.oracle.com/javase/8/docs/api/index.html?java/net/URI.html
+ return input.replaceAll("[;/?:@&=+$,\\[\\]]", "-");
+ }
Review Comment:
It seems work after this PR. I still have 2 questions:
1. Do flink or java have the utility class to remove reserved characters
before?
- I guess not only `FileMerging` feature needs to consider it. So I'm
not sure should the `uriEscape` be put here.
2. For test, did you try run a real job locally with this PR? I'm not sure
is there other issues if we run `FileMerging` feature on a standalone cluster.
--
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]
