IGNITE-8831 Fix of MarshallerMappingFileStore: Incorrect locks on files. - Fixes #4224.
Signed-off-by: Dmitriy Pavlov <dpav...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/417d31d1 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/417d31d1 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/417d31d1 Branch: refs/heads/ignite-8446 Commit: 417d31d18c6142ac6d279820d77dc1a2df4c6f9c Parents: e5af599 Author: Alexey Stelmak <spiderru5...@gmail.com> Authored: Fri Jun 22 16:43:53 2018 +0300 Committer: Dmitriy Pavlov <dpav...@apache.org> Committed: Fri Jun 22 16:43:53 2018 +0300 ---------------------------------------------------------------------- .../internal/MarshallerMappingFileStore.java | 85 ++++++++++++-------- 1 file changed, 50 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/417d31d1/modules/core/src/main/java/org/apache/ignite/internal/MarshallerMappingFileStore.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/MarshallerMappingFileStore.java b/modules/core/src/main/java/org/apache/ignite/internal/MarshallerMappingFileStore.java index 6fb1371..a01981b 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/MarshallerMappingFileStore.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/MarshallerMappingFileStore.java @@ -45,6 +45,9 @@ import org.apache.ignite.marshaller.MarshallerContext; * when a classname is requested but is not presented in local cache of {@link MarshallerContextImpl}. */ final class MarshallerMappingFileStore { + /** File lock timeout in milliseconds. */ + private static final int FILE_LOCK_TIMEOUT_MS = 5000; + /** */ private static final GridStripedLock fileLock = new GridStripedLock(32); @@ -92,14 +95,12 @@ final class MarshallerMappingFileStore { File file = new File(workDir, fileName); try (FileOutputStream out = new FileOutputStream(file)) { - FileLock fileLock = fileLock(out.getChannel(), false); - - assert fileLock != null : fileName; - try (Writer writer = new OutputStreamWriter(out, StandardCharsets.UTF_8)) { - writer.write(typeName); + try (FileLock ignored = fileLock(out.getChannel(), false)) { + writer.write(typeName); - writer.flush(); + writer.flush(); + } } } catch (IOException e) { @@ -120,11 +121,10 @@ final class MarshallerMappingFileStore { } /** - * @param platformId Platform id. - * @param typeId Type id. + * @param fileName File name. */ - String readMapping(byte platformId, int typeId) throws IgniteCheckedException { - String fileName = getFileName(platformId, typeId); + private String readMapping(String fileName) throws IgniteCheckedException { + ThreadLocalRandom rnd = null; Lock lock = fileLock(fileName); @@ -133,17 +133,30 @@ final class MarshallerMappingFileStore { try { File file = new File(workDir, fileName); - try (FileInputStream in = new FileInputStream(file)) { - FileLock fileLock = fileLock(in.getChannel(), true); + long time = 0; + + while (true) { + try (FileInputStream in = new FileInputStream(file)) { + try (BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) { + try (FileLock ignored = fileLock(in.getChannel(), true)) { + if (file.length() > 0) + return reader.readLine(); + + if (rnd == null) + rnd = ThreadLocalRandom.current(); - assert fileLock != null : fileName; + if (time == 0) + time = U.currentTimeMillis(); + else if ((U.currentTimeMillis() - time) >= FILE_LOCK_TIMEOUT_MS) + return null; - try (BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) { - return reader.readLine(); + U.sleep(rnd.nextLong(50)); + } + } + } + catch (IOException ignored) { + return null; } - } - catch (IOException ignored) { - return null; } } finally { @@ -152,6 +165,14 @@ final class MarshallerMappingFileStore { } /** + * @param platformId Platform id. + * @param typeId Type id. + */ + String readMapping(byte platformId, int typeId) throws IgniteCheckedException { + return readMapping(getFileName(platformId, typeId)); + } + + /** * Restores all mappings available in file system to marshaller context. * This method should be used only on node startup. * @@ -165,22 +186,16 @@ final class MarshallerMappingFileStore { int typeId = getTypeId(name); - try (FileInputStream in = new FileInputStream(file)) { - try (BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) { - String clsName = reader.readLine(); + String clsName = readMapping(name); - if (clsName == null) { - throw new IgniteCheckedException("Class name is null for [platformId=" + platformId + - ", typeId=" + typeId + "], marshaller mappings storage is broken. " + - "Clean up marshaller directory (<work_dir>/marshaller) and restart the node."); - } - - marshCtx.registerClassNameLocally(platformId, typeId, clsName); - } - } - catch (IOException e) { - throw new IgniteCheckedException("Reading marshaller mapping from file " + name + " failed.", e); + if (clsName == null) { + throw new IgniteCheckedException("Class name is null for [platformId=" + platformId + + ", typeId=" + typeId + "], marshaller mappings storage is broken. " + + "Clean up marshaller directory (<work_dir>/marshaller) and restart the node. File name: " + name + + ", FileSize: " + file.length()); } + + marshCtx.registerClassNameLocally(platformId, typeId, clsName); } } @@ -276,10 +291,10 @@ final class MarshallerMappingFileStore { while (true) { FileLock fileLock = ch.tryLock(0L, Long.MAX_VALUE, shared); - if (fileLock == null) - U.sleep(rnd.nextLong(50)); - else + if (fileLock != null) return fileLock; + + U.sleep(rnd.nextLong(50)); } } }
