cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bened...@apache.org
Subject [2/3] cassandra git commit: Fix TransactionLog recovery lastModified bug
Date Thu, 27 Aug 2015 16:26:11 GMT
Fix TransactionLog recovery lastModified bug

Only compares max lastModified when there are files
present for an OLD record.

patch by stefania; reviewed by benedict for CASSANDRA-10159


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/b5b32a42
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/b5b32a42
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/b5b32a42

Branch: refs/heads/trunk
Commit: b5b32a4291590f61d39877eaa3b39c40f7710218
Parents: ffe53de
Author: Stefania Alborghetti <stefania.alborghetti@datastax.com>
Authored: Tue Aug 25 15:04:52 2015 +0800
Committer: Benedict Elliott Smith <benedict@apache.org>
Committed: Thu Aug 27 17:25:18 2015 +0100

----------------------------------------------------------------------
 .../cassandra/db/lifecycle/TransactionLog.java  | 16 ++---
 .../db/lifecycle/TransactionLogTest.java        | 61 +++++++++++++-------
 2 files changed, 49 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/b5b32a42/src/java/org/apache/cassandra/db/lifecycle/TransactionLog.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/lifecycle/TransactionLog.java b/src/java/org/apache/cassandra/db/lifecycle/TransactionLog.java
index 6bc2eeb..8f83f2c 100644
--- a/src/java/org/apache/cassandra/db/lifecycle/TransactionLog.java
+++ b/src/java/org/apache/cassandra/db/lifecycle/TransactionLog.java
@@ -241,9 +241,9 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
             // Paranoid sanity checks: we create another record by looking at the files as
they are
             // on disk right now and make sure the information still matches
             Record currentRecord = Record.makeOld(files, relativeFilePath);
-            if (updateTime != currentRecord.updateTime)
+            if (updateTime != currentRecord.updateTime && currentRecord.numFiles
> 0)
             {
-                logger.error("Possible disk corruption detected for sstable [{}], record
[{}]: last update time [{}] should have been [{}]",
+                logger.error("Unexpected files detected for sstable [{}], record [{}]: last
update time [{}] should have been [{}]",
                              relativeFilePath,
                              record,
                              new Date(currentRecord.updateTime),
@@ -253,7 +253,7 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
 
             if (lastRecordIsCorrupt && currentRecord.numFiles < numFiles)
             { // if we found a corruption in the last record, then we continue only if the
number of files matches exactly.
-                logger.error("Possible disk corruption detected for sstable [{}], record
[{}]: number of files [{}] should have been [{}]",
+                logger.error("Unexpected files detected for sstable [{}], record [{}]: number
of files [{}] should have been [{}]",
                              relativeFilePath,
                              record,
                              currentRecord.numFiles,
@@ -346,7 +346,7 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
             for (Record record : records)
             {
                 if (!record.verify(parent.getFolder(), false))
-                    throw new CorruptTransactionLogException(String.format("Failed to verify
transaction %s record [%s]: possible disk corruption, aborting", parent.getId(), record),
+                    throw new CorruptTransactionLogException(String.format("Failed to verify
transaction %s record [%s]: unexpected disk state, aborting", parent.getId(), record),
                                                              this);
             }
         }
@@ -384,7 +384,7 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
                 {
                     if (!record.verify(parent.getFolder(), true))
                         throw new CorruptTransactionLogException(String.format("Last record
of transaction %s is corrupt [%s] and at least " +
-                                                                               "one previous
record does not match state on disk, possible disk corruption, aborting",
+                                                                               "one previous
record does not match state on disk, unexpected disk state, aborting",
                                                                                parent.getId(),
message),
                                                                  this);
                 }
@@ -396,7 +396,7 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
             }
             else
             {
-                throw new CorruptTransactionLogException(String.format("Non-last record of
transaction %s is corrupt [%s], possible disk corruption, aborting", parent.getId(), message),
this);
+                throw new CorruptTransactionLogException(String.format("Non-last record of
transaction %s is corrupt [%s], unexpected disk state, aborting", parent.getId(), message),
this);
             }
         }
 
@@ -989,7 +989,7 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
                     if (accumulate == null)
                         accumulate = data.removeUnfinishedLeftovers(accumulate);
                     else
-                        logger.error("Possible disk corruption: failed to read transaction
log {}", log, accumulate);
+                        logger.error("Unexpected disk state: failed to read transaction log
{}", log, accumulate);
                 }
             }
         }
@@ -1005,7 +1005,7 @@ public class TransactionLog extends Transactional.AbstractTransactional
implemen
     static final class FileLister
     {
         // The maximum number of attempts for scanning the folder
-        private static final int MAX_ATTEMPTS = 5;
+        private static final int MAX_ATTEMPTS = 10;
 
         // The delay between each attempt
         private static final int REATTEMPT_DELAY_MILLIS = 5;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/b5b32a42/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogTest.java b/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogTest.java
index 7739163..405d975 100644
--- a/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogTest.java
+++ b/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogTest.java
@@ -84,7 +84,7 @@ public class TransactionLogTest extends AbstractTransactionalTest
             final SSTableReader sstableNew;
             final TransactionLog.SSTableTidier tidier;
 
-            public Transaction(ColumnFamilyStore cfs, TransactionLog txnLogs) throws IOException
+            Transaction(ColumnFamilyStore cfs, TransactionLog txnLogs) throws IOException
             {
                 this.cfs = cfs;
                 this.txnLogs = txnLogs;
@@ -129,23 +129,23 @@ public class TransactionLogTest extends AbstractTransactionalTest
                 txnLogs.prepareToCommit();
             }
 
-            protected void assertInProgress() throws Exception
+            void assertInProgress() throws Exception
             {
                 assertFiles(txnLogs.getDataFolder(), Sets.newHashSet(Iterables.concat(sstableNew.getAllFilePaths(),
                                                                                       sstableOld.getAllFilePaths(),
                                                                                       Collections.singleton(txnLogs.getData().getLogFile().file.getPath()))));
             }
 
-            protected void assertPrepared() throws Exception
+            void assertPrepared() throws Exception
             {
             }
 
-            protected void assertAborted() throws Exception
+            void assertAborted() throws Exception
             {
                 assertFiles(txnLogs.getDataFolder(), new HashSet<>(sstableOld.getAllFilePaths()));
             }
 
-            protected void assertCommitted() throws Exception
+            void assertCommitted() throws Exception
             {
                 assertFiles(txnLogs.getDataFolder(), new HashSet<>(sstableNew.getAllFilePaths()));
             }
@@ -334,7 +334,7 @@ public class TransactionLogTest extends AbstractTransactionalTest
         transactionLog.trackNew(sstableNew);
         TransactionLog.SSTableTidier tidier = transactionLog.obsoleted(sstableOld);
 
-        Set<File> tmpFiles = Sets.newHashSet(Iterables.concat(sstableNew.getAllFilePaths().stream().map(p
-> new File(p)).collect(Collectors.toList()),
+        Set<File> tmpFiles = Sets.newHashSet(Iterables.concat(sstableNew.getAllFilePaths().stream().map(File::new).collect(Collectors.toList()),
                                                               Collections.singleton(transactionLog.getData().getLogFile().file)));
 
         sstableNew.selfRef().release();
@@ -580,7 +580,7 @@ public class TransactionLogTest extends AbstractTransactionalTest
 
         String txnFilePath = transactionLog.getData().getLogFile().file.getPath();
 
-        transactionLog.complete(null);
+        assertNull(transactionLog.complete(null));
 
         sstableOld.selfRef().release();
         sstableNew.selfRef().release();
@@ -644,7 +644,7 @@ public class TransactionLogTest extends AbstractTransactionalTest
                                  });
     }
 
-    private void testObsoletedFilesChanged(Consumer<SSTableReader> modifier) throws
IOException
+    private static void testObsoletedFilesChanged(Consumer<SSTableReader> modifier)
throws IOException
     {
         ColumnFamilyStore cfs = MockSchema.newCFS(KEYSPACE);
         SSTableReader sstableOld = sstable(cfs, 0, 128);
@@ -684,7 +684,18 @@ public class TransactionLogTest extends AbstractTransactionalTest
     }
 
     @Test
-    public void testGetTemporaryFilesSafeAfterObsoletion() throws Throwable
+    public void testGetTemporaryFilesSafeAfterObsoletion_1() throws Throwable
+    {
+        testGetTemporaryFilesSafeAfterObsoletion(true);
+    }
+
+    @Test
+    public void testGetTemporaryFilesSafeAfterObsoletion_2() throws Throwable
+    {
+        testGetTemporaryFilesSafeAfterObsoletion(false);
+    }
+
+    private void testGetTemporaryFilesSafeAfterObsoletion(boolean finishBefore) throws Throwable
     {
         ColumnFamilyStore cfs = MockSchema.newCFS(KEYSPACE);
         SSTableReader sstable = sstable(cfs, 0, 128);
@@ -695,27 +706,33 @@ public class TransactionLogTest extends AbstractTransactionalTest
 
         TransactionLog.SSTableTidier tidier = transactionLogs.obsoleted(sstable);
 
-        transactionLogs.finish();
+        if (finishBefore)
+            transactionLogs.finish();
+
         sstable.markObsolete(tidier);
         sstable.selfRef().release();
 
-        for (int i = 0; i < 1000; i++)
+        for (int i = 0; i < 100; i++)
         {
             // This should race with the asynchronous deletion of txn log files
             // It doesn't matter what it returns but it should not throw
             TransactionLog.getTemporaryFiles(cfs.metadata, dataFolder);
         }
+
+        if (!finishBefore)
+            transactionLogs.finish();
     }
 
     private static SSTableReader sstable(ColumnFamilyStore cfs, int generation, int size)
throws IOException
     {
         Directories dir = new Directories(cfs.metadata);
-        Descriptor descriptor = new Descriptor(dir.getDirectoryForNewSSTables(), cfs.keyspace.getName(),
cfs.getColumnFamilyName(), generation);
+        Descriptor descriptor = new Descriptor(dir.getDirectoryForNewSSTables(), cfs.keyspace.getName(),
cfs.getTableName(), generation);
         Set<Component> components = ImmutableSet.of(Component.DATA, Component.PRIMARY_INDEX,
Component.FILTER, Component.TOC);
         for (Component component : components)
         {
             File file = new File(descriptor.filenameFor(component));
-            file.createNewFile();
+            if (!file.exists())
+                assertTrue(file.createNewFile());
             try (RandomAccessFile raf = new RandomAccessFile(file, "rw"))
             {
                 raf.setLength(size);
@@ -725,7 +742,7 @@ public class TransactionLogTest extends AbstractTransactionalTest
         SegmentedFile dFile = new BufferedSegmentedFile(new ChannelProxy(new File(descriptor.filenameFor(Component.DATA))),
RandomAccessReader.DEFAULT_BUFFER_SIZE, 0);
         SegmentedFile iFile = new BufferedSegmentedFile(new ChannelProxy(new File(descriptor.filenameFor(Component.PRIMARY_INDEX))),
RandomAccessReader.DEFAULT_BUFFER_SIZE, 0);
 
-        SerializationHeader header = SerializationHeader.make(cfs.metadata, Collections.EMPTY_LIST);
+        SerializationHeader header = SerializationHeader.make(cfs.metadata, Collections.emptyList());
         StatsMetadata metadata = (StatsMetadata) new MetadataCollector(cfs.metadata.comparator)
                                                  .finalizeMetadata(cfs.metadata.partitioner.getClass().getCanonicalName(),
0.01f, -1, header)
                                                  .get(MetadataType.STATS);
@@ -754,14 +771,18 @@ public class TransactionLogTest extends AbstractTransactionalTest
         TransactionLog.waitForDeletions();
 
         File dir = new File(dirPath);
-        for (File file : dir.listFiles())
+        File[] files = dir.listFiles();
+        if (files != null)
         {
-            if (file.isDirectory())
-                continue;
+            for (File file : files)
+            {
+                if (file.isDirectory())
+                    continue;
 
-            String filePath = file.getPath();
-            assertTrue(filePath, expectedFiles.contains(filePath));
-            expectedFiles.remove(filePath);
+                String filePath = file.getPath();
+                assertTrue(filePath, expectedFiles.contains(filePath));
+                expectedFiles.remove(filePath);
+            }
         }
 
         if (excludeNonExistingFiles)


Mime
View raw message