You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by be...@apache.org on 2015/08/27 18:26:11 UTC
[2/3] cassandra git commit: Fix TransactionLog recovery lastModified
bug
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 <st...@datastax.com>
Authored: Tue Aug 25 15:04:52 2015 +0800
Committer: Benedict Elliott Smith <be...@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)