cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yu...@apache.org
Subject [2/3] cassandra git commit: Fix upgrade to new directory for secondary index
Date Thu, 02 Jul 2015 18:48:00 GMT
Fix upgrade to new directory for secondary index

patch by yukim; reviewed by Sam Tunnicliffe for CASSANDRA-9687


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

Branch: refs/heads/trunk
Commit: 1dbbf6040b04dfc25a5a4f0f4fac1934b5d27d93
Parents: 2357589
Author: Yuki Morishita <yukim@apache.org>
Authored: Wed Jul 1 14:48:22 2015 -0500
Committer: Yuki Morishita <yukim@apache.org>
Committed: Thu Jul 2 13:37:56 2015 -0500

----------------------------------------------------------------------
 CHANGES.txt                                     |   5 +-
 .../org/apache/cassandra/db/Directories.java    | 143 +++++++++++++------
 .../apache/cassandra/db/DirectoriesTest.java    |  98 +++++++++++--
 3 files changed, 188 insertions(+), 58 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/1dbbf604/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index bc4b57c..0b38ff0 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,3 @@
-<<<<<<< HEAD
 2.2.0-rc2
  * (cqlsh) Allow setting the initial connection timeout (CASSANDRA-9601)
  * BulkLoader has --transport-factory option but does not use it (CASSANDRA-9675)
@@ -18,12 +17,10 @@
  * Fix deprecated repair JMX API (CASSANDRA-9570)
  * Add logback metrics (CASSANDRA-9378)
  * Update and refactor ant test/test-compression to run the tests in parallel (CASSANDRA-9583)
+ * Fix upgrading to new directory for secondary index (CASSANDRA-9687)
 Merged from 2.1:
-=======
-2.1.8
  * Eliminate strong self-reference chains in sstable ref tidiers (CASSANDRA-9656)
  * Ensure StreamSession uses canonical sstable reader instances (CASSANDRA-9700) 
->>>>>>> cassandra-2.1
  * Ensure memtable book keeping is not corrupted in the event we shrink usage (CASSANDRA-9681)
  * Update internal python driver for cqlsh (CASSANDRA-9064)
  * Fix IndexOutOfBoundsException when inserting tuple with too many

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1dbbf604/src/java/org/apache/cassandra/db/Directories.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Directories.java b/src/java/org/apache/cassandra/db/Directories.java
index 4b6797f..ee8ecde 100644
--- a/src/java/org/apache/cassandra/db/Directories.java
+++ b/src/java/org/apache/cassandra/db/Directories.java
@@ -54,26 +54,33 @@ import org.apache.cassandra.utils.Pair;
 /**
  * Encapsulate handling of paths to the data files.
  *
- * Since v2.1, the directory layout is the following:
  * <pre> {@code
- *   /<path_to_data_dir>/ks/cf1-cfId/ks-cf1-ka-1-Data.db
- *                         /cf2-cfId/ks-cf2-ka-1-Data.db
+ *   /<path_to_data_dir>/ks/<cf dir>/ks-cf1-jb-1-Data.db
+ *                         /<cf dir>/la-2-Data.db
+ *                         /<cf dir>/.<index name>/ks-cf1.idx-jb-1-Data.db
+ *                         /<cf dir>/.<index name>/la-1-Data.db
  *                         ...
- * } </pre>                      
+ * } </pre>
  *
- * cfId is an hex encoded CFID.
+ * Until v2.0, {@code <cf dir>} is just column family name.
+ * Since v2.1, {@code <cf dir>} has column family ID(cfId) added to its end.
  *
- * For backward compatibility, Directories uses older directory layout if exists.
+ * SSTables from secondary indexes were put in the same directory as their parent.
+ * Since v2.2, they have their own directory under the parent directory whose name is index
name.
+ * Upon startup, those secondary index files are moved to new directory when upgrading.
+ *
+ * For backward compatibility, Directories can use directory without cfId if exists.
  *
  * In addition, more that one 'root' data directory can be specified so that
  * {@code <path_to_data_dir>} potentially represents multiple locations.
  * Note that in the case of multiple locations, the manifest for the leveled
  * compaction is only in one of the location.
  *
- * Snapshots (resp. backups) are always created along the sstables thare are
- * snapshoted (resp. backuped) but inside a subdirectory named 'snapshots'
- * (resp. backups) (and snapshots are furter inside a subdirectory of the name
- * of the snapshot).
+ * Snapshots (resp. backups) are always created along the sstables there are
+ * snapshotted (resp. backuped) but inside a subdirectory named 'snapshots'
+ * (resp. backups) (and snapshots are further inside a subdirectory of the name
+ * of the snapshot). For secondary indexes, snapshots (backups) are not created in
+ * their own directory, but are in their parent's snapshot (backup) directory.
  *
  * This class abstracts all those details from the rest of the code.
  */
@@ -176,27 +183,18 @@ public class Directories
      *
      * @param metadata metadata of ColumnFamily
      */
-    public Directories(CFMetaData metadata)
+    public Directories(final CFMetaData metadata)
     {
         this.metadata = metadata;
 
         String cfId = ByteBufferUtil.bytesToHex(ByteBufferUtil.bytes(metadata.cfId));
         int idx = metadata.cfName.indexOf(SECONDARY_INDEX_NAME_SEPARATOR);
-        // secondary indicies go in the same directory as the base cf
-        String directoryName;
-        if (idx >= 0)
-        {
-            directoryName = metadata.cfName.substring(0, idx) + "-" + cfId + File.separator
+ metadata.cfName.substring(idx);
-        }
-        else
-        {
-             directoryName = metadata.cfName + "-" + cfId;
-        }
+        String cfName = idx >= 0 ? metadata.cfName.substring(0, idx) : metadata.cfName;
+        String indexNameWithDot = idx >= 0 ? metadata.cfName.substring(idx) : null;
 
         this.dataPaths = new File[dataDirectories.length];
         // If upgraded from version less than 2.1, use existing directories
-        String oldSSTableRelativePath = join(metadata.ksName,
-                                         idx > 0 ? metadata.cfName.substring(0, idx) :
metadata.cfName);
+        String oldSSTableRelativePath = join(metadata.ksName, cfName);
         for (int i = 0; i < dataDirectories.length; ++i)
         {
             // check if old SSTable directory exists
@@ -211,12 +209,17 @@ public class Directories
         });
         if (!olderDirectoryExists)
         {
-            // use 2.1-style path names
-        	
-        	String newSSTableRelativePath = join(metadata.ksName, directoryName);
+            // use 2.1+ style
+            String newSSTableRelativePath = join(metadata.ksName, cfName + '-' + cfId);
             for (int i = 0; i < dataDirectories.length; ++i)
                 dataPaths[i] = new File(dataDirectories[i].location, newSSTableRelativePath);
         }
+        // if index, then move to its own directory
+        if (indexNameWithDot != null)
+        {
+            for (int i = 0; i < dataDirectories.length; ++i)
+                dataPaths[i] = new File(dataPaths[i], indexNameWithDot);
+        }
 
         for (File dir : dataPaths)
         {
@@ -231,6 +234,34 @@ public class Directories
                 FileUtils.handleFSError(e);
             }
         }
+
+        // if index, move existing older versioned SSTable files to new directory
+        if (indexNameWithDot != null)
+        {
+            for (File dataPath : dataPaths)
+            {
+                File[] indexFiles = dataPath.getParentFile().listFiles(new FileFilter()
+                {
+                    @Override
+                    public boolean accept(File file)
+                    {
+                        if (file.isDirectory())
+                            return false;
+
+                        Pair<Descriptor, Component> pair = SSTable.tryComponentFromFilename(file.getParentFile(),
+                                                                                        
   file.getName());
+                        return pair != null && pair.left.ksname.equals(metadata.ksName)
&& pair.left.cfname.equals(metadata.cfName);
+
+                    }
+                });
+                for (File indexFile : indexFiles)
+                {
+                    File destFile = new File(dataPath, indexFile.getName());
+                    logger.debug("Moving index file {} to {}", indexFile, destFile);
+                    FileUtils.renameWithConfirm(indexFile, destFile);
+                }
+            }
+        }
     }
 
     /**
@@ -377,6 +408,17 @@ public class Directories
         return getSnapshotDirectory(desc.directory, snapshotName);
     }
 
+    /**
+     * Returns directory to write snapshot. If directory does not exist, then one is created.
+     *
+     * If given {@code location} indicates secondary index, this will return
+     * {@code <cf dir>/snapshots/<snapshot name>/.<index name>}.
+     * Otherwise, this will return {@code <cf dir>/snapshots/<snapshot name>}.
+     *
+     * @param location base directory
+     * @param snapshotName snapshot name
+     * @return directory to write snapshot
+     */
     public static File getSnapshotDirectory(File location, String snapshotName)
     {
         if (location.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR))
@@ -391,7 +433,8 @@ public class Directories
 
     public File getSnapshotManifestFile(String snapshotName)
     {
-         return new File(getDirectoryForNewSSTables(), join(SNAPSHOT_SUBDIR, snapshotName,
"manifest.json"));
+        File snapshotDir = getSnapshotDirectory(getDirectoryForNewSSTables(), snapshotName);
+        return new File(snapshotDir, "manifest.json");
     }
 
     public static File getBackupsDirectory(Descriptor desc)
@@ -594,9 +637,11 @@ public class Directories
     public Map<String, Pair<Long, Long>> getSnapshotDetails()
     {
         final Map<String, Pair<Long, Long>> snapshotSpaceMap = new HashMap<>();
-        for (final File dir : dataPaths)
+        for (File dir : dataPaths)
         {
-            final File snapshotDir = new File(dir,SNAPSHOT_SUBDIR);
+            File snapshotDir = dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR) ?
+                                       new File(dir.getParent(), SNAPSHOT_SUBDIR) :
+                                       new File(dir, SNAPSHOT_SUBDIR);
             if (snapshotDir.exists() && snapshotDir.isDirectory())
             {
                 final File[] snapshots  = snapshotDir.listFiles();
@@ -608,9 +653,9 @@ public class Directories
                         {
                             final long sizeOnDisk = FileUtils.folderSize(snapshot);
                             final long trueSize = getTrueAllocatedSizeIn(snapshot);
-                            Pair<Long,Long> spaceUsed = snapshotSpaceMap.get(snapshot.getName());
+                            Pair<Long, Long> spaceUsed = snapshotSpaceMap.get(snapshot.getName());
                             if (spaceUsed == null)
-                                spaceUsed =  Pair.create(sizeOnDisk,trueSize);
+                                spaceUsed = Pair.create(sizeOnDisk, trueSize);
                             else
                                 spaceUsed = Pair.create(spaceUsed.left + sizeOnDisk, spaceUsed.right
+ trueSize);
                             snapshotSpaceMap.put(snapshot.getName(), spaceUsed);
@@ -622,11 +667,20 @@ public class Directories
 
         return snapshotSpaceMap;
     }
+
     public boolean snapshotExists(String snapshotName)
     {
         for (File dir : dataPaths)
         {
-            File snapshotDir = new File(dir, join(SNAPSHOT_SUBDIR, snapshotName));
+            File snapshotDir;
+            if (dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR))
+            {
+                snapshotDir = new File(dir.getParentFile(), join(SNAPSHOT_SUBDIR, snapshotName,
dir.getName()));
+            }
+            else
+            {
+                snapshotDir = new File(dir, join(SNAPSHOT_SUBDIR, snapshotName));
+            }
             if (snapshotDir.exists())
                 return true;
         }
@@ -642,8 +696,7 @@ public class Directories
             File snapshotDir = new File(dir, join(SNAPSHOT_SUBDIR, tag));
             if (snapshotDir.exists())
             {
-                if (logger.isDebugEnabled())
-                    logger.debug("Removing snapshot directory {}", snapshotDir);
+                logger.debug("Removing snapshot directory {}", snapshotDir);
                 FileUtils.deleteRecursive(snapshotDir);
             }
         }
@@ -654,18 +707,26 @@ public class Directories
     {
         for (File dir : dataPaths)
         {
-            File snapshotDir = new File(dir, join(SNAPSHOT_SUBDIR, snapshotName));
+            File snapshotDir = getSnapshotDirectory(dir, snapshotName);
             if (snapshotDir.exists())
                 return snapshotDir.lastModified();
         }
         throw new RuntimeException("Snapshot " + snapshotName + " doesn't exist");
     }
-    
+
+    /**
+     * @return total snapshot size in byte for all snapshots.
+     */
     public long trueSnapshotsSize()
     {
         long result = 0L;
         for (File dir : dataPaths)
-            result += getTrueAllocatedSizeIn(new File(dir, join(SNAPSHOT_SUBDIR)));
+        {
+            File snapshotDir = dir.getName().startsWith(SECONDARY_INDEX_NAME_SEPARATOR) ?
+                                       new File(dir.getParent(), SNAPSHOT_SUBDIR) :
+                                       new File(dir, SNAPSHOT_SUBDIR);
+            result += getTrueAllocatedSizeIn(snapshotDir);
+        }
         return result;
     }
 
@@ -673,7 +734,7 @@ public class Directories
     {
         if (!input.isDirectory())
             return 0;
-        
+
         TrueFilesSizeVisitor visitor = new TrueFilesSizeVisitor();
         try
         {
@@ -683,7 +744,7 @@ public class Directories
         {
             logger.error("Could not calculate the size of {}. {}", input, e);
         }
-    
+
         return visitor.getAllocatedSize();
     }
 
@@ -758,11 +819,11 @@ public class Directories
         private final Set<String> visited = newHashSet(); //count each file only once
         private final Set<String> alive;
 
-        public TrueFilesSizeVisitor()
+        TrueFilesSizeVisitor()
         {
             super();
             Builder<String> builder = ImmutableSet.builder();
-            for (File file: sstableLister().listFiles())
+            for (File file : sstableLister().listFiles())
                 builder.add(file.getName());
             alive = builder.build();
         }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1dbbf604/test/unit/org/apache/cassandra/db/DirectoriesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/DirectoriesTest.java b/test/unit/org/apache/cassandra/db/DirectoriesTest.java
index 080f01b..f92cecf 100644
--- a/test/unit/org/apache/cassandra/db/DirectoriesTest.java
+++ b/test/unit/org/apache/cassandra/db/DirectoriesTest.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -17,16 +17,8 @@
  */
 package org.apache.cassandra.db;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.io.*;
+import java.util.*;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
@@ -46,6 +38,7 @@ import org.apache.cassandra.io.sstable.Descriptor;
 import org.apache.cassandra.io.util.FileUtils;
 import org.apache.cassandra.utils.ByteBufferUtil;
 import org.apache.cassandra.io.FSWriteError;
+import org.apache.cassandra.utils.Pair;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -60,7 +53,11 @@ public class DirectoriesTest
     private static final String[] CFS = new String[] { "cf1", "ks" };
 
     private static final Set<CFMetaData> CFM = new HashSet<>(CFS.length);
-    private static Map<String, List<File>> files = new HashMap<String, List<File>>();
+
+    private static final CFMetaData PARENT_CFM = new CFMetaData(KS, "cf", ColumnFamilyType.Standard,
null);
+    private static final CFMetaData INDEX_CFM = new CFMetaData(KS, "cf.idx", ColumnFamilyType.Standard,
null, PARENT_CFM.cfId);
+
+    private static final Map<String, List<File>> files = new HashMap<>();
 
     @BeforeClass
     public static void beforeClass() throws IOException
@@ -122,7 +119,19 @@ public class DirectoriesTest
     private static File cfDir(CFMetaData metadata)
     {
         String cfId = ByteBufferUtil.bytesToHex(ByteBufferUtil.bytes(metadata.cfId));
-        return new File(tempDataDir, metadata.ksName + File.separator + metadata.cfName +
"-" + cfId);
+        int idx = metadata.cfName.indexOf(Directories.SECONDARY_INDEX_NAME_SEPARATOR);
+        if (idx >= 0)
+        {
+            // secondary index
+            return new File(tempDataDir,
+                            metadata.ksName + File.separator +
+                            metadata.cfName.substring(0, idx) + '-' + cfId + File.separator
+
+                            metadata.cfName.substring(idx));
+        }
+        else
+        {
+            return new File(tempDataDir, metadata.ksName + File.separator + metadata.cfName
+ '-' + cfId);
+        }
     }
 
     @Test
@@ -143,6 +152,69 @@ public class DirectoriesTest
     }
 
     @Test
+    public void testSecondaryIndexDirectories()
+    {
+        Directories parentDirectories = new Directories(PARENT_CFM);
+        Directories indexDirectories = new Directories(INDEX_CFM);
+        // secondary index has its own directory
+        for (File dir : indexDirectories.getCFDirectories())
+        {
+            assertEquals(cfDir(INDEX_CFM), dir);
+        }
+        Descriptor parentDesc = new Descriptor(parentDirectories.getDirectoryForNewSSTables(),
KS, PARENT_CFM.cfName, 0, Descriptor.Type.FINAL);
+        Descriptor indexDesc = new Descriptor(indexDirectories.getDirectoryForNewSSTables(),
KS, INDEX_CFM.cfName, 0, Descriptor.Type.FINAL);
+
+        // snapshot dir should be created under its parent's
+        File parentSnapshotDirectory = Directories.getSnapshotDirectory(parentDesc, "test");
+        File indexSnapshotDirectory = Directories.getSnapshotDirectory(indexDesc, "test");
+        assertEquals(parentSnapshotDirectory, indexSnapshotDirectory.getParentFile());
+
+        // check if snapshot directory exists
+        parentSnapshotDirectory.mkdirs();
+        assertTrue(parentDirectories.snapshotExists("test"));
+        assertTrue(indexDirectories.snapshotExists("test"));
+
+        // check their creation time
+        assertEquals(parentDirectories.snapshotCreationTime("test"),
+                     indexDirectories.snapshotCreationTime("test"));
+
+        // check true snapshot size
+        Descriptor parentSnapshot = new Descriptor(parentSnapshotDirectory, KS, PARENT_CFM.cfName,
0, Descriptor.Type.FINAL);
+        createFile(parentSnapshot.filenameFor(Component.DATA), 30);
+        Descriptor indexSnapshot = new Descriptor(indexSnapshotDirectory, KS, INDEX_CFM.cfName,
0, Descriptor.Type.FINAL);
+        createFile(indexSnapshot.filenameFor(Component.DATA), 40);
+
+        assertEquals(30, parentDirectories.trueSnapshotsSize());
+        assertEquals(40, indexDirectories.trueSnapshotsSize());
+
+        // check snapshot details
+        Map<String, Pair<Long, Long>> parentSnapshotDetail = parentDirectories.getSnapshotDetails();
+        assertTrue(parentSnapshotDetail.containsKey("test"));
+        assertEquals(30L, parentSnapshotDetail.get("test").right.longValue());
+
+        Map<String, Pair<Long, Long>> indexSnapshotDetail = indexDirectories.getSnapshotDetails();
+        assertTrue(indexSnapshotDetail.containsKey("test"));
+        assertEquals(40L, indexSnapshotDetail.get("test").right.longValue());
+
+        // check backup directory
+        File parentBackupDirectory = Directories.getBackupsDirectory(parentDesc);
+        File indexBackupDirectory = Directories.getBackupsDirectory(indexDesc);
+        assertEquals(parentBackupDirectory, indexBackupDirectory.getParentFile());
+    }
+
+    private File createFile(String fileName, int size)
+    {
+        File newFile = new File(fileName);
+        try (FileOutputStream writer = new FileOutputStream(newFile))
+        {
+            writer.write(new byte[size]);
+            writer.flush();
+        }
+        catch (IOException ignore) {}
+        return newFile;
+    }
+
+    @Test
     public void testSSTableLister()
     {
         for (CFMetaData cfm : CFM)


Mime
View raw message