Zakelly commented on code in PR #25310:
URL: https://github.com/apache/flink/pull/25310#discussion_r1756001947
##########
flink-state-backends/flink-statebackend-forst/src/main/java/org/apache/flink/state/forst/fs/ForStFlinkFileSystem.java:
##########
@@ -101,6 +142,12 @@ public boolean rename(Path src, Path dst) throws
IOException {
// The rename is not atomic for ForSt. Some FileSystems e.g. HDFS, OSS
does not allow a
// renaming if the target already exists. So, we delete the target
before attempting the
// rename.
+ if (isLocal(src) && isLocal(dst)) {
+ return localFS.rename(src, dst);
+ } else if (isLocal(src) || isLocal(dst)) {
+ throw new IOException("Rename between local and remote is not
supported.");
+ }
+
Review Comment:
Should `isLocal` consider the `fileFilter`?
##########
flink-state-backends/flink-statebackend-forst/src/main/java/org/apache/flink/state/forst/fs/ForStFlinkFileSystem.java:
##########
@@ -42,10 +45,16 @@ public class ForStFlinkFileSystem extends FileSystem {
// TODO: make it configurable
private static final int DEFAULT_INPUT_STREAM_CAPACITY = 32;
+ private static final Map<String, String> remoteLocalMapping = new
ConcurrentHashMap<>();
+ private static final Function<String, Boolean> miscFileFilter = s ->
!s.endsWith(".sst");
+ private static final FileSystem localFS = FileSystem.getLocalFileSystem();
+
private final FileSystem delegateFS;
+ private final Function<String, Boolean> fileFilter;
Review Comment:
nit. How about renaming this to `localFileFilter`
##########
flink-state-backends/flink-statebackend-forst/src/main/java/org/apache/flink/state/forst/fs/ForStFlinkFileSystem.java:
##########
@@ -61,6 +70,16 @@ public static FileSystem get(URI uri) throws IOException {
return new ForStFlinkFileSystem(FileSystem.get(uri));
}
+ /**
+ * Setup local base path for corresponding remote base path.
+ *
+ * @param remoteBasePath the remote base path.
+ * @param localBasePath the local base path.
+ */
+ public static void setupLocalBasePath(String remoteBasePath, String
localBasePath) {
+ remoteLocalMapping.put(remoteBasePath, localBasePath);
Review Comment:
Is there any way removing the map entry? I'm worried this would keep growing.
--
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]
