You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ty...@apache.org on 2014/03/19 00:14:03 UTC

[1/2] git commit: Pick unused generations when loading new sstables

Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.1 8440bc550 -> 11a961842


Pick unused generations when loading new sstables

patch by Tyler Hobbs; reviewed by Jonathan Ellis for CASSANDRA-6514


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

Branch: refs/heads/cassandra-2.1
Commit: 9269cb83ce0bd45eff0f42611d5b5cd4415cba31
Parents: 75ff51e
Author: Tyler Hobbs <ty...@datastax.com>
Authored: Tue Mar 18 18:03:06 2014 -0500
Committer: Tyler Hobbs <ty...@datastax.com>
Committed: Tue Mar 18 18:03:06 2014 -0500

----------------------------------------------------------------------
 CHANGES.txt                                     |  2 +
 .../apache/cassandra/db/ColumnFamilyStore.java  | 26 +++++--
 .../cassandra/db/ColumnFamilyStoreTest.java     | 79 ++++++++++++++++++++
 3 files changed, 101 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/9269cb83/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 9caca38..4741475 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,6 @@
 2.0.7
+ * Lower chances for losing new SSTables during nodetool refresh and
+   ColumnFamilyStore.loadNewSSTables (CASSANDRA-6514)
  * Add support for DELETE ... IF EXISTS to CQL3 (CASSANDRA-5708)
  * Update hadoop_cql3_word_count example (CASSANDRA-6793)
  * Fix handling of RejectedExecution in sync Thrift server (CASSANDRA-6788)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/9269cb83/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index eed44a4..5c3eb19 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -652,12 +652,20 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
                 continue;
             }
 
-            Descriptor newDescriptor = new Descriptor(descriptor.version,
-                                                      descriptor.directory,
-                                                      descriptor.ksname,
-                                                      descriptor.cfname,
-                                                      fileIndexGenerator.incrementAndGet(),
-                                                      false);
+            // Increment the generation until we find a filename that doesn't exist. This is needed because the new
+            // SSTables that are being loaded might already use these generation numbers.
+            Descriptor newDescriptor;
+            do
+            {
+                newDescriptor = new Descriptor(descriptor.version,
+                                               descriptor.directory,
+                                               descriptor.ksname,
+                                               descriptor.cfname,
+                                               fileIndexGenerator.incrementAndGet(),
+                                               false);
+            }
+            while (new File(newDescriptor.filenameFor(Component.DATA)).exists());
+
             logger.info("Renaming new SSTable {} to {}", descriptor, newDescriptor);
             SSTableWriter.rename(descriptor, newDescriptor, entry.getValue());
 
@@ -2431,4 +2439,10 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
         Pair<ReplayPosition, Long> truncationRecord = SystemKeyspace.getTruncationRecords().get(metadata.cfId);
         return truncationRecord == null ? Long.MIN_VALUE : truncationRecord.right;
     }
+
+    @VisibleForTesting
+    void resetFileIndexGenerator()
+    {
+        fileIndexGenerator.set(0);
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/9269cb83/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
index 2edf6a8..5fc006b 100644
--- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
+++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
@@ -29,6 +29,8 @@ import java.util.concurrent.Future;
 import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.io.util.FileUtils;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.junit.Test;
@@ -1744,6 +1746,83 @@ public class ColumnFamilyStoreTest extends SchemaLoader
         assert sstables.containsKey(sstable1.descriptor);
     }
 
+    @Test
+    public void testLoadNewSSTablesAvoidsOverwrites() throws Throwable
+    {
+        String ks = "Keyspace1";
+        String cf = "Standard1";
+        ColumnFamilyStore cfs = Keyspace.open(ks).getColumnFamilyStore(cf);
+        cfs.truncateBlocking();
+        SSTableDeletingTask.waitForDeletions();
+
+        final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf);
+        Directories dir = Directories.create(ks, cf);
+
+        // clear old SSTables (probably left by CFS.clearUnsafe() calls in other tests)
+        for (Map.Entry<Descriptor, Set<Component>> entry : dir.sstableLister().list().entrySet())
+        {
+            for (Component component : entry.getValue())
+            {
+                FileUtils.delete(entry.getKey().filenameFor(component));
+            }
+        }
+
+        // sanity check
+        int existingSSTables = dir.sstableLister().list().keySet().size();
+        assert existingSSTables == 0 : String.format("%d SSTables unexpectedly exist", existingSSTables);
+
+        ByteBuffer key = bytes("key");
+
+        SSTableSimpleWriter writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(),
+                                                              cfmeta, StorageService.getPartitioner());
+        writer.newRow(key);
+        writer.addColumn(bytes("col"), bytes("val"), 1);
+        writer.close();
+
+        writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(),
+                                         cfmeta, StorageService.getPartitioner());
+        writer.newRow(key);
+        writer.addColumn(bytes("col"), bytes("val"), 1);
+        writer.close();
+
+        Set<Integer> generations = new HashSet<>();
+        for (Descriptor descriptor : dir.sstableLister().list().keySet())
+            generations.add(descriptor.generation);
+
+        // we should have two generations: [1, 2]
+        assertEquals(2, generations.size());
+        assertTrue(generations.contains(1));
+        assertTrue(generations.contains(2));
+
+        assertEquals(0, cfs.getSSTables().size());
+
+        // start the generation counter at 1 again (other tests have incremented it already)
+        cfs.resetFileIndexGenerator();
+
+        boolean incrementalBackupsEnabled = DatabaseDescriptor.isIncrementalBackupsEnabled();
+        try
+        {
+            // avoid duplicate hardlinks to incremental backups
+            DatabaseDescriptor.setIncrementalBackupsEnabled(false);
+            cfs.loadNewSSTables();
+        }
+        finally
+        {
+            DatabaseDescriptor.setIncrementalBackupsEnabled(incrementalBackupsEnabled);
+        }
+
+        assertEquals(2, cfs.getSSTables().size());
+        generations = new HashSet<>();
+        for (Descriptor descriptor : dir.sstableLister().list().keySet())
+            generations.add(descriptor.generation);
+
+        // normally they would get renamed to generations 1 and 2, but since those filenames already exist,
+        // they get skipped and we end up with generations 3 and 4
+        assertEquals(2, generations.size());
+        assertTrue(generations.contains(3));
+        assertTrue(generations.contains(4));
+    }
+
     private ColumnFamilyStore prepareMultiRangeSlicesTest(int valueSize, boolean flush) throws Throwable
     {
         String keyspaceName = "Keyspace1";


[2/2] git commit: Merge branch 'cassandra-2.0' into cassandra-2.1

Posted by ty...@apache.org.
Merge branch 'cassandra-2.0' into cassandra-2.1

Conflicts:
	CHANGES.txt
	src/java/org/apache/cassandra/db/ColumnFamilyStore.java


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

Branch: refs/heads/cassandra-2.1
Commit: 11a96184277ef775fe15af0ea3e5bad51dca7a11
Parents: 8440bc5 9269cb8
Author: Tyler Hobbs <ty...@datastax.com>
Authored: Tue Mar 18 18:13:47 2014 -0500
Committer: Tyler Hobbs <ty...@datastax.com>
Committed: Tue Mar 18 18:13:47 2014 -0500

----------------------------------------------------------------------
 CHANGES.txt                                     |  2 +
 .../apache/cassandra/db/ColumnFamilyStore.java  | 26 +++++--
 .../cassandra/db/ColumnFamilyStoreTest.java     | 79 ++++++++++++++++++++
 3 files changed, 101 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/11a96184/CHANGES.txt
----------------------------------------------------------------------
diff --cc CHANGES.txt
index f03614c,4741475..9f66506
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@@ -1,29 -1,6 +1,31 @@@
 -2.0.7
 +2.1.0-beta2
 + * Eliminate possibility of CL segment appearing twice in active list 
 +   (CASSANDRA-6557)
 + * Apply DONTNEED fadvise to commitlog segments (CASSANDRA-6759)
 + * Switch CRC component to Adler and include it for compressed sstables 
 +   (CASSANDRA-4165)
 + * Allow cassandra-stress to set compaction strategy options (CASSANDRA-6451)
 + * Add broadcast_rpc_address option to cassandra.yaml (CASSANDRA-5899)
 + * Auto reload GossipingPropertyFileSnitch config (CASSANDRA-5897)
 + * Fix overflow of memtable_total_space_in_mb (CASSANDRA-6573)
 + * Fix ABTC NPE and apply update function correctly (CASSANDRA-6692)
 + * Allow nodetool to use a file or prompt for password (CASSANDRA-6660)
 + * Fix AIOOBE when concurrently accessing ABSC (CASSANDRA-6742)
 + * Fix assertion error in ALTER TYPE RENAME (CASSANDRA-6705)
 + * Scrub should not always clear out repaired status (CASSANDRA-5351)
 + * Improve handling of range tombstone for wide partitions (CASSANDRA-6446)
 + * Fix ClassCastException for compact table with composites (CASSANDRA-6738)
 + * Fix potentially repairing with wrong nodes (CASSANDRA-6808)
 + * Change caching option syntax (CASSANDRA-6745)
 + * Fix stress to do proper counter reads (CASSANDRA-6835)
 + * Fix help message for stress counter_write (CASSANDRA-6824)
 + * Fix stress smart Thrift client to pick servers correctly (CASSANDRA-6848)
 + * Add logging levels (minimal, normal or verbose) to stress tool (CASSANDRA-6849)
 + * Fix race condition in Batch CLE (CASSANDRA-6860)
 + * Improve cleanup/scrub/upgradesstables failure handling (CASSANDRA-6774)
 +Merged from 2.0:
+  * Lower chances for losing new SSTables during nodetool refresh and
+    ColumnFamilyStore.loadNewSSTables (CASSANDRA-6514)
   * Add support for DELETE ... IF EXISTS to CQL3 (CASSANDRA-5708)
   * Update hadoop_cql3_word_count example (CASSANDRA-6793)
   * Fix handling of RejectedExecution in sync Thrift server (CASSANDRA-6788)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/11a96184/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index e116574,5c3eb19..845352d
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@@ -2716,8 -2440,9 +2724,14 @@@ public class ColumnFamilyStore implemen
          return truncationRecord == null ? Long.MIN_VALUE : truncationRecord.right;
      }
  
 +    public long trueSnapshotsSize()
 +    {
 +        return directories.trueSnapshotsSize();
 +    }
++
+     @VisibleForTesting
+     void resetFileIndexGenerator()
+     {
+         fileIndexGenerator.set(0);
+     }
  }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/11a96184/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
index 9d25f78,5fc006b..0fc82ff
--- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
+++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
@@@ -1727,6 -1746,83 +1729,83 @@@ public class ColumnFamilyStoreTest exte
          assert sstables.containsKey(sstable1.descriptor);
      }
  
+     @Test
+     public void testLoadNewSSTablesAvoidsOverwrites() throws Throwable
+     {
+         String ks = "Keyspace1";
+         String cf = "Standard1";
+         ColumnFamilyStore cfs = Keyspace.open(ks).getColumnFamilyStore(cf);
+         cfs.truncateBlocking();
+         SSTableDeletingTask.waitForDeletions();
+ 
+         final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf);
 -        Directories dir = Directories.create(ks, cf);
++        Directories dir = new Directories(cfs.metadata);
+ 
+         // clear old SSTables (probably left by CFS.clearUnsafe() calls in other tests)
+         for (Map.Entry<Descriptor, Set<Component>> entry : dir.sstableLister().list().entrySet())
+         {
+             for (Component component : entry.getValue())
+             {
+                 FileUtils.delete(entry.getKey().filenameFor(component));
+             }
+         }
+ 
+         // sanity check
+         int existingSSTables = dir.sstableLister().list().keySet().size();
+         assert existingSSTables == 0 : String.format("%d SSTables unexpectedly exist", existingSSTables);
+ 
+         ByteBuffer key = bytes("key");
+ 
 -        SSTableSimpleWriter writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(),
++        SSTableSimpleWriter writer = new SSTableSimpleWriter(dir.getDirectoryForCompactedSSTables(),
+                                                               cfmeta, StorageService.getPartitioner());
+         writer.newRow(key);
+         writer.addColumn(bytes("col"), bytes("val"), 1);
+         writer.close();
+ 
 -        writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(),
++        writer = new SSTableSimpleWriter(dir.getDirectoryForCompactedSSTables(),
+                                          cfmeta, StorageService.getPartitioner());
+         writer.newRow(key);
+         writer.addColumn(bytes("col"), bytes("val"), 1);
+         writer.close();
+ 
+         Set<Integer> generations = new HashSet<>();
+         for (Descriptor descriptor : dir.sstableLister().list().keySet())
+             generations.add(descriptor.generation);
+ 
+         // we should have two generations: [1, 2]
+         assertEquals(2, generations.size());
+         assertTrue(generations.contains(1));
+         assertTrue(generations.contains(2));
+ 
+         assertEquals(0, cfs.getSSTables().size());
+ 
+         // start the generation counter at 1 again (other tests have incremented it already)
+         cfs.resetFileIndexGenerator();
+ 
+         boolean incrementalBackupsEnabled = DatabaseDescriptor.isIncrementalBackupsEnabled();
+         try
+         {
+             // avoid duplicate hardlinks to incremental backups
+             DatabaseDescriptor.setIncrementalBackupsEnabled(false);
+             cfs.loadNewSSTables();
+         }
+         finally
+         {
+             DatabaseDescriptor.setIncrementalBackupsEnabled(incrementalBackupsEnabled);
+         }
+ 
+         assertEquals(2, cfs.getSSTables().size());
+         generations = new HashSet<>();
+         for (Descriptor descriptor : dir.sstableLister().list().keySet())
+             generations.add(descriptor.generation);
+ 
+         // normally they would get renamed to generations 1 and 2, but since those filenames already exist,
+         // they get skipped and we end up with generations 3 and 4
+         assertEquals(2, generations.size());
+         assertTrue(generations.contains(3));
+         assertTrue(generations.contains(4));
+     }
+ 
      private ColumnFamilyStore prepareMultiRangeSlicesTest(int valueSize, boolean flush) throws Throwable
      {
          String keyspaceName = "Keyspace1";