From f0c619b866bd5295ad9359278de73964a98fc430 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 8 Sep 2016 15:19:07 +0200 Subject: [PATCH] Add a file rename method to the PageCache interface We need this since only the page cache is aware of any custom IO subsystem configurations we might have, and renaming files is an operation that is used in a fair number of places. --- .../io/fs/DefaultFileSystemAbstraction.java | 9 +- .../io/fs/DelegateFileSystemAbstraction.java | 5 +- .../neo4j/io/fs/FileSystemAbstraction.java | 2 +- .../main/java/org/neo4j/io/fs/FileUtils.java | 33 +--- .../org/neo4j/io/pagecache/PageCache.java | 14 ++ .../io/pagecache/PageSwapperFactory.java | 19 +++ .../impl/CannotMoveMappedFileException.java | 31 ++++ .../impl/SingleFilePageSwapperFactory.java | 7 + .../impl/muninn/MuninnPageCache.java | 37 ++++- .../fs/AdversarialFileSystemAbstraction.java | 4 +- .../pagecache/AdversarialPageCache.java | 12 ++ .../DelegatingFileSystemAbstraction.java | 4 +- .../EphemeralFileSystemAbstraction.java | 27 ++-- .../mockfs/LimitedFilesystemAbstraction.java | 4 +- .../SelectiveFileSystemAbstraction.java | 4 +- .../io/pagecache/DelegatingPageCache.java | 8 + .../org/neo4j/io/pagecache/PageCacheTest.java | 149 +++++++++++++++++- .../kernel/impl/index/IndexConfigStore.java | 9 +- .../java/org/neo4j/ha/TestBranchedData.java | 2 +- 19 files changed, 308 insertions(+), 72 deletions(-) create mode 100644 community/io/src/main/java/org/neo4j/io/pagecache/impl/CannotMoveMappedFileException.java diff --git a/community/io/src/main/java/org/neo4j/io/fs/DefaultFileSystemAbstraction.java b/community/io/src/main/java/org/neo4j/io/fs/DefaultFileSystemAbstraction.java index 326506afda408..2f876be6252fa 100644 --- a/community/io/src/main/java/org/neo4j/io/fs/DefaultFileSystemAbstraction.java +++ b/community/io/src/main/java/org/neo4j/io/fs/DefaultFileSystemAbstraction.java @@ -135,14 +135,9 @@ public void deleteRecursively( File directory ) throws IOException } @Override - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { - if ( copyOptions.length > 0 ) - { - Files.move( from.toPath(), to.toPath(), copyOptions ); - return true; // will throw if failure - } - return FileUtils.renameFile( from, to ); + Files.move( from.toPath(), to.toPath(), copyOptions ); } @Override diff --git a/community/io/src/main/java/org/neo4j/io/fs/DelegateFileSystemAbstraction.java b/community/io/src/main/java/org/neo4j/io/fs/DelegateFileSystemAbstraction.java index c6959202ca353..4178daa230f81 100644 --- a/community/io/src/main/java/org/neo4j/io/fs/DelegateFileSystemAbstraction.java +++ b/community/io/src/main/java/org/neo4j/io/fs/DelegateFileSystemAbstraction.java @@ -38,11 +38,9 @@ import java.nio.file.StandardCopyOption; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.stream.Stream; -import static java.nio.file.StandardCopyOption.ATOMIC_MOVE; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; /** @@ -170,10 +168,9 @@ public void deleteRecursively( File directory ) throws IOException } @Override - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { Files.move( path( from ), path( to ), copyOptions ); - return true; } @Override diff --git a/community/io/src/main/java/org/neo4j/io/fs/FileSystemAbstraction.java b/community/io/src/main/java/org/neo4j/io/fs/FileSystemAbstraction.java index 34a98e02cccde..aac3b83e4548d 100644 --- a/community/io/src/main/java/org/neo4j/io/fs/FileSystemAbstraction.java +++ b/community/io/src/main/java/org/neo4j/io/fs/FileSystemAbstraction.java @@ -58,7 +58,7 @@ public interface FileSystemAbstraction void deleteRecursively( File directory ) throws IOException; - boolean move( File from, File to, CopyOption... copyOptions ) throws IOException; + void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException; File[] listFiles( File directory ); diff --git a/community/io/src/main/java/org/neo4j/io/fs/FileUtils.java b/community/io/src/main/java/org/neo4j/io/fs/FileUtils.java index fe8a3e60fd32b..ea621848c770e 100644 --- a/community/io/src/main/java/org/neo4j/io/fs/FileUtils.java +++ b/community/io/src/main/java/org/neo4j/io/fs/FileUtils.java @@ -42,6 +42,7 @@ import java.nio.channels.SeekableByteChannel; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.CopyOption; import java.nio.file.DirectoryStream; import java.nio.file.FileVisitResult; import java.nio.file.Files; @@ -120,7 +121,7 @@ public static boolean deleteFile( File file ) * Utility method that moves a file from its current location to the * new target location. If rename fails (for example if the target is * another disk) a copy/delete will be performed instead. This is not a rename, - * use {@link #renameFile(File, File)} instead. + * use {@link #renameFile(File, File, CopyOption...)} instead. * * @param toMove The File object to move. * @param target Target file to move to. @@ -161,7 +162,7 @@ public static void moveFile( File toMove, File target ) throws IOException * Utility method that moves a file from its current location to the * provided target directory. If rename fails (for example if the target is * another disk) a copy/delete will be performed instead. This is not a rename, - * use {@link #renameFile(File, File)} instead. + * use {@link #renameFile(File, File, CopyOption...)} instead. * * @param toMove The File object to move. * @param targetDirectory the destination directory @@ -181,33 +182,9 @@ public static File moveFileToDirectory( File toMove, File targetDirectory ) thro return target; } - public static boolean renameFile( File srcFile, File renameToFile ) throws IOException + public static void renameFile( File srcFile, File renameToFile, CopyOption... copyOptions ) throws IOException { - if ( !srcFile.exists() ) - { - throw new FileNotFoundException( "Source file[" + srcFile.getName() + "] not found" ); - } - if ( renameToFile.exists() ) - { - throw new FileNotFoundException( "Target file[" + renameToFile.getName() + "] already exists" ); - } - if ( !renameToFile.getParentFile().isDirectory() ) - { - throw new FileNotFoundException( "Target directory[" + renameToFile.getParent() + "] does not exists" ); - } - int count = 0; - boolean renamed; - do - { - renamed = srcFile.renameTo( renameToFile ); - if ( !renamed ) - { - count++; - waitAndThenTriggerGC(); - } - } - while ( !renamed && count <= WINDOWS_RETRY_COUNT ); - return renamed; + Files.move( srcFile.toPath(), renameToFile.toPath(), copyOptions ); } public static void truncateFile( SeekableByteChannel fileChannel, long position ) diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/PageCache.java b/community/io/src/main/java/org/neo4j/io/pagecache/PageCache.java index 2fe882edc23be..ab223940bb628 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/PageCache.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/PageCache.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; +import java.nio.file.CopyOption; import java.nio.file.OpenOption; import java.nio.file.StandardOpenOption; import java.util.Optional; @@ -94,4 +95,17 @@ public interface PageCache extends AutoCloseable /** The max number of cached pages. */ int maxCachedPages(); + + /** + * Move the file from the given source path, to the given target path. + * + * Both files have to be unmapped when performing the move, otherwise an exception will be thrown. + * + * @param sourceFile The name of the file to move. + * @param targetFile The new name of the file after the move. + * @param copyOptions Options to modify the behaviour of the move in possibly platform specific ways. In particular, + * {@link java.nio.file.StandardCopyOption#REPLACE_EXISTING} may be used to overwrite any existing file at the + * target path name, instead of throwing an exception. + */ + void moveFile( File sourceFile, File targetFile, CopyOption... copyOptions ) throws IOException; } diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/PageSwapperFactory.java b/community/io/src/main/java/org/neo4j/io/pagecache/PageSwapperFactory.java index 79ae35d13b721..eb87ae5e0013b 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/PageSwapperFactory.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/PageSwapperFactory.java @@ -21,6 +21,8 @@ import java.io.File; import java.io.IOException; +import java.nio.file.CopyOption; +import java.nio.file.Path; import org.neo4j.io.fs.FileSystemAbstraction; @@ -97,4 +99,21 @@ PageSwapper createPageSwapper( * durable regardless of which method that does the forcing. */ void syncDevice() throws IOException; + + /** + * Move the file by the given source file name, to the path indicated by the given target file name. + * + * The provided list of {@link CopyOption CopyOptions} can be used to modify and influence platform specific + * behaviour. In particular, {@link java.nio.file.StandardCopyOption#REPLACE_EXISTING} may be used to overwrite any + * existing file at the target path name, instead of throwing an exception. + * + * Implementors are free to assume that neither file name will be mapped by the page cache at the time of the move, + * and thus the move will see no interference from concurrent IO operations. + * @param sourceFile The existing file to move. + * @param targetFile The desired new path of the source file. This file should not exist, unless + * {@link java.nio.file.StandardCopyOption#REPLACE_EXISTING} is given as a {@code copyOption}. + * @param copyOptions Options to modify the behaviour of the move in possibly platform specific ways. + * @see java.nio.file.Files#move(Path, Path, CopyOption...) + */ + void moveUnopenedFile( File sourceFile, File targetFile, CopyOption... copyOptions ) throws IOException; } diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/CannotMoveMappedFileException.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/CannotMoveMappedFileException.java new file mode 100644 index 0000000000000..6b6e753948d3c --- /dev/null +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/CannotMoveMappedFileException.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2002-2016 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.io.pagecache.impl; + +import java.io.File; +import java.io.IOException; + +public class CannotMoveMappedFileException extends IOException +{ + public CannotMoveMappedFileException( File file ) + { + super( "Cannot move mapped file: " + file ); + } +} diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/SingleFilePageSwapperFactory.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/SingleFilePageSwapperFactory.java index a3051d9042c19..b171cd55a2160 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/SingleFilePageSwapperFactory.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/SingleFilePageSwapperFactory.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; +import java.nio.file.CopyOption; import java.nio.file.NoSuchFileException; import org.neo4j.io.fs.FileSystemAbstraction; @@ -70,6 +71,12 @@ public void syncDevice() // Nothing do to, since we `fsync` files individually in `force()`. } + @Override + public void moveUnopenedFile( File sourceFile, File targetFile, CopyOption... copyOptions ) throws IOException + { + fs.renameFile( sourceFile, targetFile, copyOptions ); + } + @Override public String implementationName() { diff --git a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCache.java b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCache.java index 68214d1b91b4d..b1886db5ab889 100644 --- a/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCache.java +++ b/community/io/src/main/java/org/neo4j/io/pagecache/impl/muninn/MuninnPageCache.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; +import java.nio.file.CopyOption; import java.nio.file.OpenOption; import java.nio.file.StandardOpenOption; import java.util.Arrays; @@ -37,6 +38,7 @@ import org.neo4j.io.pagecache.PageCacheOpenOptions; import org.neo4j.io.pagecache.PageSwapperFactory; import org.neo4j.io.pagecache.PagedFile; +import org.neo4j.io.pagecache.impl.CannotMoveMappedFileException; import org.neo4j.io.pagecache.tracing.EvictionEvent; import org.neo4j.io.pagecache.tracing.EvictionRunEvent; import org.neo4j.io.pagecache.tracing.FlushEventOpportunity; @@ -370,7 +372,17 @@ public synchronized Optional getExistingMapping( File file ) throws I ensureThreadsInitialised(); file = file.getCanonicalFile(); + MuninnPagedFile pagedFile = tryGetMappingOrNull( file ); + if ( pagedFile != null ) + { + pagedFile.incrementRefCount(); + return Optional.of( pagedFile ); + } + return Optional.empty(); + } + private MuninnPagedFile tryGetMappingOrNull( File file ) throws IOException + { FileMapping current = mappedFiles; // find an existing mapping @@ -378,15 +390,32 @@ public synchronized Optional getExistingMapping( File file ) throws I { if ( current.file.equals( file ) ) { - MuninnPagedFile pagedFile = current.pagedFile; - pagedFile.incrementRefCount(); - return Optional.of( current.pagedFile ); + return current.pagedFile; } current = current.next; } // no mapping exists - return Optional.empty(); + return null; + } + + @Override + public synchronized void moveFile( File sourceFile, File targetFile, CopyOption... copyOptions ) + throws IOException + { + sourceFile = sourceFile.getCanonicalFile(); + targetFile = targetFile.getCanonicalFile(); + throwIfMapped( sourceFile ); + throwIfMapped( targetFile ); + swapperFactory.moveUnopenedFile( sourceFile, targetFile, copyOptions ); + } + + private void throwIfMapped( File file ) throws IOException + { + if ( tryGetMappingOrNull( file ) != null ) + { + throw new CannotMoveMappedFileException( file ); + } } /** diff --git a/community/io/src/test/java/org/neo4j/adversaries/fs/AdversarialFileSystemAbstraction.java b/community/io/src/test/java/org/neo4j/adversaries/fs/AdversarialFileSystemAbstraction.java index 9d34fd5067cfb..a7e2865cec0a4 100644 --- a/community/io/src/test/java/org/neo4j/adversaries/fs/AdversarialFileSystemAbstraction.java +++ b/community/io/src/test/java/org/neo4j/adversaries/fs/AdversarialFileSystemAbstraction.java @@ -69,10 +69,10 @@ public StoreChannel open( File fileName, String mode ) throws IOException return AdversarialFileChannel.wrap( (StoreFileChannel) delegate.open( fileName, mode ), adversary ); } - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { adversary.injectFailure( FileNotFoundException.class, SecurityException.class ); - return delegate.move( from, to, copyOptions ); + delegate.renameFile( from, to, copyOptions ); } public OutputStream openAsOutputStream( File fileName, boolean append ) throws IOException diff --git a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialPageCache.java b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialPageCache.java index c9f9b0af739d7..b94d1011e4c2d 100644 --- a/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialPageCache.java +++ b/community/io/src/test/java/org/neo4j/adversaries/pagecache/AdversarialPageCache.java @@ -24,6 +24,8 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.file.CopyOption; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.OpenOption; import java.nio.file.StandardOpenOption; import java.util.Objects; @@ -33,6 +35,7 @@ import org.neo4j.io.pagecache.IOLimiter; import org.neo4j.io.pagecache.PageCache; import org.neo4j.io.pagecache.PagedFile; +import org.neo4j.io.pagecache.impl.CannotMoveMappedFileException; /** * A {@linkplain PageCache page cache} that wraps another page cache and an {@linkplain Adversary adversary} to provide @@ -112,4 +115,13 @@ public int maxCachedPages() { return delegate.maxCachedPages(); } + + @Override + public void moveFile( File sourceFile, File targetFile, CopyOption... copyOptions ) + throws IOException + { + adversary.injectFailure( CannotMoveMappedFileException.class, FileAlreadyExistsException.class, + IOException.class, SecurityException.class ); + delegate.moveFile( sourceFile, targetFile, copyOptions ); + } } diff --git a/community/io/src/test/java/org/neo4j/graphdb/mockfs/DelegatingFileSystemAbstraction.java b/community/io/src/test/java/org/neo4j/graphdb/mockfs/DelegatingFileSystemAbstraction.java index 82b4ad757e4e0..be1d93445a4da 100644 --- a/community/io/src/test/java/org/neo4j/graphdb/mockfs/DelegatingFileSystemAbstraction.java +++ b/community/io/src/test/java/org/neo4j/graphdb/mockfs/DelegatingFileSystemAbstraction.java @@ -86,9 +86,9 @@ public long lastModifiedTime( File file ) throws IOException } @Override - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { - return delegate.move( from, to, copyOptions ); + delegate.renameFile( from, to, copyOptions ); } @Override diff --git a/community/io/src/test/java/org/neo4j/graphdb/mockfs/EphemeralFileSystemAbstraction.java b/community/io/src/test/java/org/neo4j/graphdb/mockfs/EphemeralFileSystemAbstraction.java index 3cf6773607610..2c130903a2487 100644 --- a/community/io/src/test/java/org/neo4j/graphdb/mockfs/EphemeralFileSystemAbstraction.java +++ b/community/io/src/test/java/org/neo4j/graphdb/mockfs/EphemeralFileSystemAbstraction.java @@ -43,6 +43,8 @@ import java.nio.file.CopyOption; import java.nio.file.StandardCopyOption; import java.time.Clock; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.NoSuchFileException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -69,6 +71,7 @@ import static java.lang.Math.max; import static java.lang.Math.min; +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static java.util.Arrays.asList; public class EphemeralFileSystemAbstraction implements FileSystemAbstraction @@ -381,30 +384,30 @@ public void deleteRecursively( File directory ) throws IOException } @Override - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { from = canonicalFile( from ); to = canonicalFile( to ); - boolean replaceExisting = false; - for ( CopyOption op : copyOptions ) + if ( !files.containsKey( from ) ) { - if ( op == StandardCopyOption.REPLACE_EXISTING ) - { - replaceExisting = true; - } + throw new NoSuchFileException( "'" + from + "' doesn't exist" ); } - if ( !files.containsKey( from ) ) + boolean replaceExisting = false; + for ( CopyOption copyOption : copyOptions ) + { + replaceExisting |= copyOption == REPLACE_EXISTING; + } + if ( files.containsKey( to ) && !replaceExisting ) { - throw new IOException( "'" + from + "' doesn't exist" ); + throw new FileAlreadyExistsException( "'" + to + "' already exists" ); } - if ( !replaceExisting && files.containsKey( to ) ) + if ( !isDirectory( to.getParentFile() ) ) { - throw new IOException( "'" + to + "' already exists" ); + throw new NoSuchFileException( "Target directory[" + to.getParent() + "] does not exists" ); } files.put( to, files.remove( from ) ); - return true; } @Override diff --git a/community/io/src/test/java/org/neo4j/graphdb/mockfs/LimitedFilesystemAbstraction.java b/community/io/src/test/java/org/neo4j/graphdb/mockfs/LimitedFilesystemAbstraction.java index 054afe7f991ef..fab14c52e6482 100644 --- a/community/io/src/test/java/org/neo4j/graphdb/mockfs/LimitedFilesystemAbstraction.java +++ b/community/io/src/test/java/org/neo4j/graphdb/mockfs/LimitedFilesystemAbstraction.java @@ -89,10 +89,10 @@ public void mkdirs( File fileName ) throws IOException } @Override - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { ensureHasSpace(); - return super.move( from, to, copyOptions ); + super.renameFile( from, to, copyOptions ); } public void runOutOfDiskSpace( boolean outOfSpace ) diff --git a/community/io/src/test/java/org/neo4j/graphdb/mockfs/SelectiveFileSystemAbstraction.java b/community/io/src/test/java/org/neo4j/graphdb/mockfs/SelectiveFileSystemAbstraction.java index 6c2a930ea0be1..a14abb1886f7d 100644 --- a/community/io/src/test/java/org/neo4j/graphdb/mockfs/SelectiveFileSystemAbstraction.java +++ b/community/io/src/test/java/org/neo4j/graphdb/mockfs/SelectiveFileSystemAbstraction.java @@ -126,9 +126,9 @@ public void deleteRecursively( File directory ) throws IOException } @Override - public boolean move( File from, File to, CopyOption... copyOptions ) throws IOException + public void renameFile( File from, File to, CopyOption... copyOptions ) throws IOException { - return chooseFileSystem( from ).move( from, to, copyOptions ); + chooseFileSystem( from ).renameFile( from, to, copyOptions ); } @Override diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/DelegatingPageCache.java b/community/io/src/test/java/org/neo4j/io/pagecache/DelegatingPageCache.java index 52041eb1df98a..162d34a61bbea 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/DelegatingPageCache.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/DelegatingPageCache.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; +import java.nio.file.CopyOption; import java.nio.file.OpenOption; import java.util.Optional; @@ -59,6 +60,13 @@ public int maxCachedPages() return delegate.maxCachedPages(); } + @Override + public void moveFile( File sourceFile, File targetFile, CopyOption... copyOptions ) + throws IOException + { + delegate.moveFile( sourceFile, targetFile, copyOptions ); + } + public void flushAndForce( IOLimiter limiter ) throws IOException { delegate.flushAndForce( limiter ); diff --git a/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java b/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java index afed0ec9f469d..2ce0e718c8a4e 100644 --- a/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java +++ b/community/io/src/test/java/org/neo4j/io/pagecache/PageCacheTest.java @@ -33,6 +33,7 @@ import java.nio.channels.ClosedChannelException; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.NoSuchFileException; import java.nio.file.OpenOption; import java.nio.file.StandardOpenOption; @@ -57,6 +58,7 @@ import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.fs.StoreChannel; +import org.neo4j.io.pagecache.impl.CannotMoveMappedFileException; import org.neo4j.io.pagecache.impl.SingleFilePageSwapperFactory; import org.neo4j.io.pagecache.randomharness.Record; import org.neo4j.io.pagecache.randomharness.StandardRecordFormat; @@ -67,8 +69,9 @@ import static java.lang.Long.toHexString; import static java.lang.System.currentTimeMillis; - +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.hamcrest.Matchers.both; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; @@ -4690,4 +4693,148 @@ public void writingToClosedWritableByteChannelMustThrow() throws Exception fail( "That read should have thrown" ); } } + + @Test( expected = CannotMoveMappedFileException.class ) + public void moveFileMustThrowIfSourceFileIsMapped() throws Exception + { + File a = file( "a" ); + File b = file( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + try ( PagedFile ignore = pageCache.map( a, filePageSize ) ) + { + pageCache.moveFile( a, b ); + } + } + + @Test( expected = CannotMoveMappedFileException.class ) + public void moveFileMustThrowIfTargetFileIsMapped() throws Exception + { + File a = file( "a" ); + File b = existingFile( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + try ( PagedFile ignore = pageCache.map( b, filePageSize ) ) + { + pageCache.moveFile( a, b ); + } + } + + @Test( expected = FileAlreadyExistsException.class ) + public void moveFileMustThrowIfTargetFileExistsAndReplaceExistingIsNotEnabled() throws Exception + { + File a = file( "a" ); + File b = existingFile( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + pageCache.moveFile( a, b ); + } + + @Test + public void moveFileMustThrowIfTargetDirectoryDoesNotExist() throws Exception + { + File a = file( "a" ); + File b = new File( file( "targetDir" ), "targetFile" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + try + { + pageCache.moveFile( a, b ); + fail( "pageCache.moveFile should have thrown" ); + } + catch ( NoSuchFileException e ) + { + assertThat( e.getMessage(), containsString( "targetDir" ) ); + } + } + + @Test( expected = NoSuchFileException.class ) + public void moveFileMustThrowIfSourceFileDoesNotExist() throws Exception + { + File a = file( "doesNotExist" ); + File b = file( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + pageCache.moveFile( a, b ); + } + + @Test + public void fileMustBeMappableAtNewLocationAfterMove() throws Exception + { + File a = file( "a" ); + File b = file( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + pageCache.moveFile( a, b ); + pageCache.map( b, filePageSize ).close(); + } + + @Test( expected = NoSuchFileException.class ) + public void sourceFileMustNotBeMappableAfterMove() throws Exception + { + File a = file( "a" ); + File b = file( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + pageCache.moveFile( a, b ); + pageCache.map( a, filePageSize ); + fail( "pageCache.map should have thrown" ); + } + + @Test + public void sourceFileContentsMustBeUnchangedAfterMove() throws Exception + { + File a = file( "a" ); + File b = file( "b" ); + generateFileWithRecords( a, recordCount, recordSize ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + pageCache.moveFile( a, b ); + verifyRecordsInFile( b, recordCount ); + } + + @Test + public void sourceFileContentsMustBeUnchangedAfterMoveEvenIfReplacingExistingFile() throws Exception + { + File a = file( "a" ); + File b = existingFile( "b" ); + generateFileWithRecords( a, recordCount, recordSize ); + generateFileWithRecords( b, recordCount + recordsPerFilePage, recordSize ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + + // Fill 'b' with random data + try ( PagedFile pf = pageCache.map( b, filePageSize ); + PageCursor cursor = pf.io( 0, PF_SHARED_WRITE_LOCK | PF_NO_GROW ) ) + { + ThreadLocalRandom rng = ThreadLocalRandom.current(); + while ( cursor.next() ) + { + int pageSize = cursor.getCurrentPageSize(); + for ( int i = 0; i < pageSize; i++ ) + { + cursor.putByte( i, (byte) rng.nextInt() ); + } + } + } + + // Do the move + pageCache.moveFile( a, b, REPLACE_EXISTING ); + + // Then verify that the old random data we put in 'b' has been replaced with the contents of 'a' + verifyRecordsInFile( b, recordCount ); + } + + @Test + public void moveFileMustCanonicaliseSourceFile() throws Exception + { + File a = new File( new File( file( "a" ), "poke" ), ".." ); + File b = file( "b" ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + // File 'a' should canonicalise from 'a/poke/..' to 'a', which is a file that exists. + // Thus, this should not throw a NoSuchFileException. + pageCache.moveFile( a, b ); + } + + @Test + public void moveFileMustCanonicaliseTargetFile() throws Exception + { + File a = file( "a" ); + File b = new File( new File( file( "b" ), "poke" ), ".." ); + getPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL ); + // File 'b' should canonicalise from 'b/poke/..' to 'b', which is a file that doesn't exists. + // Thus, this should not throw a FileAlreadyExistsException. + pageCache.moveFile( a, b ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/IndexConfigStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/IndexConfigStore.java index 4b5e9012f578f..2ccd244dfd4d2 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/IndexConfigStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/IndexConfigStore.java @@ -264,9 +264,9 @@ private void write() fileSystem.deleteFile( oldFile ); try { - if ( fileSystem.fileExists( file ) && !fileSystem.move( file, oldFile ) ) + if ( fileSystem.fileExists( file ) ) { - throw new RuntimeException( "Couldn't rename " + file + " -> " + oldFile ); + fileSystem.renameFile( file, oldFile ); } } catch ( IOException e ) @@ -277,10 +277,7 @@ private void write() // Rename the .tmp file to the current name try { - if ( !fileSystem.move( tmpFile, this.file ) ) - { - throw new RuntimeException( "Couldn't rename " + tmpFile + " -> " + file ); - } + fileSystem.renameFile( tmpFile, this.file ); } catch ( IOException e ) { diff --git a/enterprise/ha/src/test/java/org/neo4j/ha/TestBranchedData.java b/enterprise/ha/src/test/java/org/neo4j/ha/TestBranchedData.java index bab38cd771a18..fb872d931f7c2 100644 --- a/enterprise/ha/src/test/java/org/neo4j/ha/TestBranchedData.java +++ b/enterprise/ha/src/test/java/org/neo4j/ha/TestBranchedData.java @@ -307,7 +307,7 @@ private long moveAwayToLookLikeOldBranchedDirectory() throws IOException String fileName = file.getName(); if ( !fileName.equals( StoreLogService.INTERNAL_LOG_NAME ) && !file.getName().startsWith( "branched-" ) ) { - assertTrue( FileUtils.renameFile( file, new File( branchDir, file.getName() ) ) ); + FileUtils.renameFile( file, new File( branchDir, file.getName() ) ); } } return timestamp;