paul8263 commented on a change in pull request #16108: URL: https://github.com/apache/flink/pull/16108#discussion_r766417762
########## File path: flink-core/src/main/java/org/apache/flink/util/FileLock.java ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.util; + +import org.apache.flink.annotation.Internal; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; + +/** A file lock used for avoiding race condition among multiple threads/processes. */ +@Internal +public class FileLock { + private static final String TEMP_DIR = System.getProperty("java.io.tmpdir"); + private final File file; + private FileOutputStream outputStream; + private java.nio.channels.FileLock lock; + + /** + * Initialize a FileLock using a file located at fullPath. + * + * @param fullPath The path of the locking file + */ + public FileLock(String fullPath) { + Preconditions.checkNotNull(fullPath, "fullPath should not be null"); + Path path = Paths.get(fullPath); + String normalizedFileName = normalizeFileName(path.getFileName().toString()); + if (normalizedFileName.isEmpty()) { + throw new IllegalArgumentException("There are no legal characters in the file name"); + } + this.file = + path.getParent() == null + ? new File(normalizedFileName) + : new File(path.getParent().toString(), normalizedFileName); + } + + /** + * Initialize a FileLock using a file located at parentDir/fileName. + * + * @param parentDir The parent dir of the locking file + * @param fileName The name of the locking file + */ + public FileLock(String parentDir, String fileName) { + Preconditions.checkNotNull(parentDir, "parentDir should not be null"); + Preconditions.checkNotNull(fileName, "fileName should not be null"); + this.file = new File(parentDir, normalizeFileName(fileName)); + } + + /** + * Initialize a FileLock using a file located inside temp folder. + * + * @param fileName The name of the locking file + * @return The initialized FileLock + */ + public static FileLock inTempFolder(String fileName) { + return new FileLock(TEMP_DIR, fileName); + } + + /** + * Check whether the locking file exists in the file system. Create it if it does not exist. + * Then create a FileOutputStream for it. + * + * @throws IOException If the file path is invalid or the parent dir does not exist + */ + private void init() throws IOException { + if (!this.file.exists()) { + this.file.createNewFile(); + } + outputStream = new FileOutputStream(this.file); + } + + /** + * Try to acquire a lock on the locking file. This method immediately returns whenever the lock + * is acquired or not. + * + * @return True if successfully acquired the lock + * @throws IOException If the file path is invalid + */ + public boolean tryLock() throws IOException { + if (outputStream == null) { + init(); + } + try { + lock = outputStream.getChannel().tryLock(); + } catch (Exception e) { + return false; + } + + return lock != null; + } + + /** + * Release the file lock. + * + * @throws IOException If the FileChannel is closed + */ + public void unlock() throws IOException { + if (lock != null && lock.channel().isOpen()) { + lock.release(); + } + } Review comment: Hi @AHeise , Thank you very much for you advice. After refactored tests which call `getAvailablePort`, I think there might be another issue. In some unit tests such as flink-clients/src/test/java/org/apache/flink/client/program/ClientTest.java, the port is allocated in setup method annotated with `@Before`, which is called before every single test. It might be difficult to use a try block across two method invocation. After I reviewed the documentation of maven surefire plugin parallel test execution in https://maven.apache.org/surefire/maven-surefire-plugin/examples/fork-options-and-parallel-execution.html, it says we could use `@net.jcip.annotations.NotThreadSafe` to make it running in a single thread. Or even set the `forkCount` parameter to limit the JVM process that execute tests. I think by doing this we can mitigate this issue, but the unit test will cost much longer time. Whether we can use it needs further discussion. -- 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.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
