From a4307ba848076bbfe6810e4d9faa17f48d8da91d Mon Sep 17 00:00:00 2001 From: MishaDemianenko Date: Wed, 27 Sep 2017 15:28:34 +0200 Subject: [PATCH] Address PR comments --- .../recovery/CorruptedLogsTruncator.java | 13 ++++--- .../neo4j/kernel/recovery/LogTailScanner.java | 38 ++++++++++--------- .../RecoveryCorruptedTransactionLogIT.java | 2 +- .../recovery/CorruptedLogsTruncatorTest.java | 6 +-- .../kernel/recovery/LogTailScannerTest.java | 4 +- 5 files changed, 32 insertions(+), 31 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/recovery/CorruptedLogsTruncator.java b/community/kernel/src/main/java/org/neo4j/kernel/recovery/CorruptedLogsTruncator.java index 8c638cfab628..39aecac35787 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/recovery/CorruptedLogsTruncator.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/recovery/CorruptedLogsTruncator.java @@ -32,13 +32,14 @@ import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.fs.StoreChannel; import org.neo4j.kernel.impl.transaction.log.LogPosition; +import org.neo4j.kernel.impl.transaction.log.PhysicalLogFile; import org.neo4j.kernel.impl.transaction.log.PhysicalLogFiles; import static java.lang.String.format; /** * Transaction log truncator used during recovery to truncate all the logs after some specified position, that - * recovery threats as corrupted or non-readable. + * recovery treats as corrupted or non-readable. * Transaction log file specified by provided log position will be truncated to provided length, any * subsequent files will be removed. * Any removed or modified log content will be stored in separate corruption logs archive for further analysis and as @@ -46,8 +47,8 @@ */ public class CorruptedLogsTruncator { - public static final String CORRUPTED_TX_LOGS_FOLDER_NAME = "corrupted-tx-logs"; - private static final String LOG_FILE_ARCHIVE_PATTERN = "corrupted-logs-%d-%d-%d.zip"; + public static final String CORRUPTED_TX_LOGS_BASE_NAME = "corrupted-" + PhysicalLogFile.DEFAULT_NAME; + private static final String LOG_FILE_ARCHIVE_PATTERN = CORRUPTED_TX_LOGS_BASE_NAME + "-%d-%d-%d.zip"; private final File storeDir; private final PhysicalLogFiles logFiles; @@ -72,7 +73,7 @@ public void truncate( LogPosition positionAfterLastRecoveredTransaction ) throws long recoveredTransactionLogVersion = positionAfterLastRecoveredTransaction.getLogVersion(); long recoveredTransactionOffset = positionAfterLastRecoveredTransaction.getByteOffset(); if ( isRecoveredLogCorrupted( recoveredTransactionLogVersion, recoveredTransactionOffset ) || - isNotLastLogFile( recoveredTransactionLogVersion ) ) + haveMoreRecentLogFiles( recoveredTransactionLogVersion ) ) { backupCorruptedContent( recoveredTransactionLogVersion, recoveredTransactionOffset ); truncateLogFiles( recoveredTransactionLogVersion, recoveredTransactionOffset ); @@ -124,7 +125,7 @@ private void backupCorruptedContent( long recoveredTransactionLogVersion, long r private File getArchiveFile( long recoveredTransactionLogVersion, long recoveredTransactionOffset ) throws IOException { - File corruptedLogsFolder = new File( storeDir, CORRUPTED_TX_LOGS_FOLDER_NAME ); + File corruptedLogsFolder = new File( storeDir, CORRUPTED_TX_LOGS_BASE_NAME ); fs.mkdirs( corruptedLogsFolder ); return new File( corruptedLogsFolder, format( LOG_FILE_ARCHIVE_PATTERN, recoveredTransactionLogVersion, recoveredTransactionOffset, @@ -155,7 +156,7 @@ private void copyTransactionLogContent( long logFileIndex, long logOffset, ZipOu destination.closeEntry(); } - private boolean isNotLastLogFile( long recoveredTransactionLogVersion ) + private boolean haveMoreRecentLogFiles( long recoveredTransactionLogVersion ) { return logFiles.getHighestLogVersion() > recoveredTransactionLogVersion; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/recovery/LogTailScanner.java b/community/kernel/src/main/java/org/neo4j/kernel/recovery/LogTailScanner.java index 833d55bc0e7a..3fe25f63beb6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/recovery/LogTailScanner.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/recovery/LogTailScanner.java @@ -189,7 +189,6 @@ protected LogTailInformation checkpointTailInformation( long highestLogVersion, protected ExtractedTransactionRecord extractFirstTxIdAfterPosition( LogPosition initialPosition, long maxLogVersion ) throws IOException { LogPosition currentPosition = initialPosition; - ExtractedTransactionRecord transactionRecord = new ExtractedTransactionRecord(); while ( currentPosition.getLogVersion() <= maxLogVersion ) { LogVersionedStoreChannel storeChannel = PhysicalLogFile.tryOpenForVersion( logFiles, fileSystem, @@ -207,8 +206,7 @@ protected ExtractedTransactionRecord extractFirstTxIdAfterPosition( LogPosition LogEntry entry = cursor.get(); if ( entry instanceof LogEntryCommit ) { - transactionRecord.setId( ((LogEntryCommit) entry).getTxId() ); - return transactionRecord; + return new ExtractedTransactionRecord( ((LogEntryCommit) entry).getTxId() ); } } } @@ -216,8 +214,7 @@ protected ExtractedTransactionRecord extractFirstTxIdAfterPosition( LogPosition catch ( Throwable t ) { monitor.corruptedLogFile( currentPosition.getLogVersion(), t ); - transactionRecord.setFailure( true ); - return transactionRecord; + return new ExtractedTransactionRecord( true ); } finally { @@ -227,7 +224,7 @@ protected ExtractedTransactionRecord extractFirstTxIdAfterPosition( LogPosition currentPosition = LogPosition.start( currentPosition.getLogVersion() + 1 ); } - return transactionRecord; + return new ExtractedTransactionRecord(); } /** @@ -260,33 +257,38 @@ public LogTailInformation getTailInformation() throws UnderlyingStorageException static class ExtractedTransactionRecord { - private long id; - private boolean failure; + private final long id; + private final boolean failure; ExtractedTransactionRecord() { - id = NO_TRANSACTION_ID; - failure = false; + this( NO_TRANSACTION_ID, false ); } - public long getId() + ExtractedTransactionRecord( long txId ) { - return id; + this( txId, false ); } - public void setId( long id ) + ExtractedTransactionRecord( boolean failure ) { - this.id = id; + this( NO_TRANSACTION_ID, failure ); } - public boolean isFailure() + private ExtractedTransactionRecord( long txId, boolean failure ) { - return failure; + this.id = txId; + this.failure = failure; } - public void setFailure( boolean failure ) + public long getId() { - this.failure = failure; + return id; + } + + public boolean isFailure() + { + return failure; } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/RecoveryCorruptedTransactionLogIT.java b/community/kernel/src/test/java/org/neo4j/kernel/RecoveryCorruptedTransactionLogIT.java index 397934e81b04..a8a9a47253cb 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/RecoveryCorruptedTransactionLogIT.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/RecoveryCorruptedTransactionLogIT.java @@ -397,7 +397,7 @@ public void repetitiveRecoveryIfCorruptedLogsWithCheckpoints() throws IOExceptio assertThat( numberOfRecoveredTransactions, Matchers.greaterThanOrEqualTo( 0 ) ); } - File corruptedLogArchives = new File( storeDir, CorruptedLogsTruncator.CORRUPTED_TX_LOGS_FOLDER_NAME ); + File corruptedLogArchives = new File( storeDir, CorruptedLogsTruncator.CORRUPTED_TX_LOGS_BASE_NAME ); assertThat( corruptedLogArchives.listFiles(), not( emptyArray() ) ); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/recovery/CorruptedLogsTruncatorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/recovery/CorruptedLogsTruncatorTest.java index 3f5c520825ee..69cd3b54d048 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/recovery/CorruptedLogsTruncatorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/recovery/CorruptedLogsTruncatorTest.java @@ -108,7 +108,7 @@ public void pruneAndArchiveLastLog() throws IOException assertEquals( TOTAL_NUMBER_OF_LOG_FILES, storeDir.listFiles( LogFiles.FILENAME_FILTER ).length ); assertEquals( byteOffset, highestLogFile.length() ); - File corruptedLogsDirectory = new File( storeDir, CorruptedLogsTruncator.CORRUPTED_TX_LOGS_FOLDER_NAME ); + File corruptedLogsDirectory = new File( storeDir, CorruptedLogsTruncator.CORRUPTED_TX_LOGS_BASE_NAME ); assertTrue( corruptedLogsDirectory.exists() ); File[] files = corruptedLogsDirectory.listFiles(); assertEquals( 1, files.length ); @@ -139,7 +139,7 @@ public void pruneAndArchiveMultipleLogs() throws IOException assertEquals( 6, storeDir.listFiles( LogFiles.FILENAME_FILTER ).length ); assertEquals( byteOffset, highestCorrectLogFile.length() ); - File corruptedLogsDirectory = new File( storeDir, CorruptedLogsTruncator.CORRUPTED_TX_LOGS_FOLDER_NAME ); + File corruptedLogsDirectory = new File( storeDir, CorruptedLogsTruncator.CORRUPTED_TX_LOGS_BASE_NAME ); assertTrue( corruptedLogsDirectory.exists() ); File[] files = corruptedLogsDirectory.listFiles(); assertEquals( 1, files.length ); @@ -176,7 +176,7 @@ private void checkEntryNameAndSize( ZipFile zipFile, String entryName, int expec private void checkArchiveName( long highestLogVersion, long byteOffset, File corruptedLogsArchive ) { String name = corruptedLogsArchive.getName(); - assertTrue( name.startsWith( "corrupted-logs-" + highestLogVersion + "-" + byteOffset ) ); + assertTrue( name.startsWith( "corrupted-neostore.transaction.db-" + highestLogVersion + "-" + byteOffset ) ); assertTrue( FilenameUtils.isExtension( name, "zip" ) ); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/recovery/LogTailScannerTest.java b/community/kernel/src/test/java/org/neo4j/kernel/recovery/LogTailScannerTest.java index 7e1005574e6e..0b54df8a47aa 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/recovery/LogTailScannerTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/recovery/LogTailScannerTest.java @@ -608,9 +608,7 @@ private static class FirstTxIdConfigurableTailScanner extends LogTailScanner @Override protected ExtractedTransactionRecord extractFirstTxIdAfterPosition( LogPosition initialPosition, long maxLogVersion ) { - ExtractedTransactionRecord record = new ExtractedTransactionRecord(); - record.setId( txId ); - return record; + return new ExtractedTransactionRecord( txId ); } } }