accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From els...@apache.org
Subject [1/4] git commit: ACCUMULO-3215 Ensure tablet directories are absolute, not relative
Date Thu, 16 Oct 2014 00:11:08 GMT
Repository: accumulo
Updated Branches:
  refs/heads/1.6 0348e3ea3 -> 6ad16a9c5
  refs/heads/master a21d2d3cf -> f118b2211


ACCUMULO-3215 Ensure tablet directories are absolute, not relative

Move 'c-' into a constant, make the computed srv:dir in ImportTable
an absolute directory (choosing a volume as per policy for each tablet),
and add a test to make sure the path is constructed correctly.

ImportExportIT was extended to check that srv:dir, in addition to file:,
is an absolute path.


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

Branch: refs/heads/1.6
Commit: 6ad16a9c50f48a98def23e5720218659a410dd08
Parents: 0348e3e
Author: Josh Elser <elserj@apache.org>
Authored: Wed Oct 15 15:14:25 2014 -0400
Committer: Josh Elser <elserj@apache.org>
Committed: Wed Oct 15 19:31:29 2014 -0400

----------------------------------------------------------------------
 .../org/apache/accumulo/core/Constants.java     |  5 +-
 .../accumulo/server/util/MetadataTableUtil.java |  2 +-
 .../accumulo/gc/SimpleGarbageCollector.java     |  2 +-
 server/master/pom.xml                           |  5 ++
 .../accumulo/master/tableOps/ImportTable.java   | 34 ++++++++++--
 .../master/tableOps/ImportTableTest.java        | 54 ++++++++++++++++++++
 .../org/apache/accumulo/tserver/Tablet.java     | 46 ++++++++---------
 .../apache/accumulo/test/ImportExportIT.java    | 35 ++++++++++---
 8 files changed, 147 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/core/src/main/java/org/apache/accumulo/core/Constants.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/Constants.java b/core/src/main/java/org/apache/accumulo/core/Constants.java
index 15cccf4..2a570f4 100644
--- a/core/src/main/java/org/apache/accumulo/core/Constants.java
+++ b/core/src/main/java/org/apache/accumulo/core/Constants.java
@@ -56,7 +56,7 @@ public class Constants {
   public static final String ZMONITOR_LOCK = ZMONITOR + "/lock";
   public static final String ZMONITOR_HTTP_ADDR = ZMONITOR + "/http_addr";
   public static final String ZMONITOR_LOG4J_ADDR = ZMONITOR + "/log4j_addr";
-  
+
   public static final String ZCONFIG = "/config";
 
   public static final String ZTSERVERS = "/tservers";
@@ -88,6 +88,9 @@ public class Constants {
 
   public static final String BULK_PREFIX = "b-";
 
+  public static final String CLONE_PREFIX = "c-";
+  public static final byte[] CLONE_PREFIX_BYTES = CLONE_PREFIX.getBytes(UTF8);
+
   // this affects the table client caching of metadata
   public static final int SCAN_BATCH_SIZE = 1000;
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
index f978439..f908d25 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
@@ -881,7 +881,7 @@ public class MetadataTableUtil {
       Mutation m = new Mutation(k.getRow());
       m.putDelete(k.getColumnFamily(), k.getColumnQualifier());
       String dir = volumeManager.choose(ServerConstants.getTablesDirs()) + "/" + tableId
-          + new String(FastFormat.toZeroPaddedString(dirCount++, 8, 16, "/c-".getBytes(Constants.UTF8)));
+          + new String(FastFormat.toZeroPaddedString(dirCount++, 8, 16, Constants.CLONE_PREFIX_BYTES));
       TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value(dir.getBytes(Constants.UTF8)));
       bw.addMutation(m);
     }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
----------------------------------------------------------------------
diff --git a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
index 4617299..d04303b 100644
--- a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
+++ b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
@@ -421,7 +421,7 @@ public class SimpleGarbageCollector implements Iface {
                   TableState tableState = TableManager.getInstance().getTableState(tableId);
                   if (tableState != null && tableState != TableState.DELETING) {
                     // clone directories don't always exist
-                    if (!tabletDir.startsWith("c-"))
+                    if (!tabletDir.startsWith(Constants.CLONE_PREFIX))
                       log.debug("File doesn't exist: " + fullPath);
                   }
                 } else {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/server/master/pom.xml
----------------------------------------------------------------------
diff --git a/server/master/pom.xml b/server/master/pom.xml
index bfd6b68..0268680 100644
--- a/server/master/pom.xml
+++ b/server/master/pom.xml
@@ -77,6 +77,11 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.easymock</groupId>
+      <artifactId>easymock</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
       <scope>test</scope>

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java
----------------------------------------------------------------------
diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java
b/server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java
index e57c689..8de6bce 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java
@@ -222,7 +222,9 @@ class PopulateMetadataTable extends MasterRepo {
       log.info("importDir is " + tableInfo.importDir);
 
       // This is a directory already prefixed with proper volume information e.g. hdfs://localhost:8020/path/to/accumulo/tables/...
-      String bulkDir = tableInfo.importDir;
+      final String bulkDir = tableInfo.importDir;
+
+      final String[] tableDirs = ServerConstants.getTablesDirs();
 
       ZipEntry zipEntry;
       while ((zipEntry = zis.getNextEntry()) != null) {
@@ -260,15 +262,28 @@ class PopulateMetadataTable extends MasterRepo {
             }
 
             if (m == null) {
+              // Make a unique directory inside the table's dir. Cannot import multiple tables
into one table, so don't need to use unique allocator
+              String tabletDir = new String(FastFormat.toZeroPaddedString(dirCount++, 8,
16, Constants.CLONE_PREFIX_BYTES), Constants.UTF8);
+
+              // Build up a full hdfs://localhost:8020/accumulo/tables/$id/c-XXXXXXX
+              String absolutePath = getClonedTabletDir(master, tableDirs, tabletDir);
+
               m = new Mutation(metadataRow);
-              TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value(FastFormat.toZeroPaddedString(dirCount++,
8, 16, "/c-".getBytes(Constants.UTF8))));
+              TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value(absolutePath.getBytes(Constants.UTF8)));
               currentRow = metadataRow;
             }
 
             if (!currentRow.equals(metadataRow)) {
               mbw.addMutation(m);
+
+              // Make a unique directory inside the table's dir. Cannot import multiple tables
into one table, so don't need to use unique allocator
+              String tabletDir = new String(FastFormat.toZeroPaddedString(dirCount++, 8,
16, Constants.CLONE_PREFIX_BYTES), Constants.UTF8);
+
+              // Build up a full hdfs://localhost:8020/accumulo/tables/$id/c-XXXXXXX
+              String absolutePath = getClonedTabletDir(master, tableDirs, tabletDir);
+
               m = new Mutation(metadataRow);
-              TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value(FastFormat.toZeroPaddedString(dirCount++,
8, 16, "/c-".getBytes(Constants.UTF8))));
+              TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.put(m, new Value(absolutePath.getBytes(Constants.UTF8)));
             }
 
             m.put(key.getColumnFamily(), cq, val);
@@ -303,6 +318,19 @@ class PopulateMetadataTable extends MasterRepo {
     }
   }
 
+  /**
+   * Given options for tables (across multiple volumes), construct an absolute path using
the unique name within the chosen volume
+   *
+   * @return An absolute, unique path for the imported table
+   */
+  protected String getClonedTabletDir(Master master, String[] tableDirs, String tabletDir)
{
+    // We can try to spread out the tablet dirs across all volumes
+    String tableDir = master.getFileSystem().choose(tableDirs);
+
+    // Build up a full hdfs://localhost:8020/accumulo/tables/$id/c-XXXXXXX
+    return tableDir + "/" + tableInfo.tableId + "/" + tabletDir;
+  }
+
   @Override
   public void undo(long tid, Master environment) throws Exception {
     MetadataTableUtil.deleteTable(tableInfo.tableId, false, SystemCredentials.get(), environment.getMasterLock());

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/server/master/src/test/java/org/apache/accumulo/master/tableOps/ImportTableTest.java
----------------------------------------------------------------------
diff --git a/server/master/src/test/java/org/apache/accumulo/master/tableOps/ImportTableTest.java
b/server/master/src/test/java/org/apache/accumulo/master/tableOps/ImportTableTest.java
new file mode 100644
index 0000000..31f6bde
--- /dev/null
+++ b/server/master/src/test/java/org/apache/accumulo/master/tableOps/ImportTableTest.java
@@ -0,0 +1,54 @@
+/*
+ * 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 regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.accumulo.master.tableOps;
+
+import org.apache.accumulo.master.Master;
+import org.apache.accumulo.server.fs.VolumeManager;
+import org.easymock.EasyMock;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class ImportTableTest {
+
+  @Test
+  public void testTabletDir() {
+    Master master = EasyMock.createMock(Master.class);
+    VolumeManager volumeManager = EasyMock.createMock(VolumeManager.class);
+    ImportedTableInfo iti = new ImportedTableInfo();
+    iti.tableId = "5";
+
+    // Different volumes with different paths
+    String[] tableDirs = new String[] {"hdfs://nn1:8020/apps/accumulo1/tables", "hdfs://nn2:8020/applications/accumulo/tables"};
+    // This needs to be unique WRT the importtable command
+    String tabletDir = "/c-00000001";
+
+    EasyMock.expect(master.getFileSystem()).andReturn(volumeManager);
+    // Choose the 2nd element
+    EasyMock.expect(volumeManager.choose(tableDirs)).andReturn(tableDirs[1]);
+
+    EasyMock.replay(master, volumeManager);
+
+    PopulateMetadataTable pmt = new PopulateMetadataTable(iti);
+    Assert.assertEquals(tableDirs[1] + "/" + iti.tableId + "/" + tabletDir, pmt.getClonedTabletDir(master,
tableDirs, tabletDir));
+
+    EasyMock.verify(master, volumeManager);
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java
----------------------------------------------------------------------
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java
index 53469d7..de930aa 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java
@@ -157,16 +157,16 @@ import com.google.common.annotations.VisibleForTesting;
  * We need to be able to have the master tell a tabletServer to
  * close this file, and the tablet server to handle all pending client reads
  * before closing
- * 
+ *
  */
 
 /**
- * 
+ *
  * this class just provides an interface to read from a MapFile mostly takes care of reporting
start and end keys
- * 
+ *
  * need this because a single row extent can have multiple columns this manages all the columns
(each handled by a store) for a single row-extent
- * 
- * 
+ *
+ *
  */
 
 public class Tablet {
@@ -422,7 +422,7 @@ public class Tablet {
 
   private long lastFlushID = -1;
   private long lastCompactID = -1;
-  
+
   private static class CompactionWaitInfo {
     long flushID = -1;
     long compactionID = -1;
@@ -523,7 +523,7 @@ public class Tablet {
     }
 
     if (files == null) {
-      if (tabletDir.getName().startsWith("c-"))
+      if (tabletDir.getName().startsWith(Constants.CLONE_PREFIX))
         log.debug("Tablet " + extent + " had no dir, creating " + tabletDir); // its a clone
dir...
       else
         log.warn("Tablet " + extent + " had no dir, creating " + tabletDir);
@@ -833,7 +833,7 @@ public class Tablet {
               log.warn("Target map file already exist " + newDatafile);
               fs.deleteRecursively(newDatafile.path());
             }
-            
+
             rename(fs, tmpDatafile.path(), newDatafile.path());
           }
           break;
@@ -966,7 +966,7 @@ public class Tablet {
         // rename before putting in metadata table, so files in metadata table should
         // always exist
         rename(fs, tmpDatafile.path(), newDatafile.path());
-        
+
         if (dfv.getNumEntries() == 0) {
           fs.deleteRecursively(newDatafile.path());
         }
@@ -1081,7 +1081,7 @@ public class Tablet {
     this.tabletDirectory = tabletDirectory;
     this.logId = logId;
     this.location = location;
-    this.datafileManager = datafileManager; 
+    this.datafileManager = datafileManager;
   }
 
   private Tablet(TabletServer tabletServer, Text location, KeyExtent extent, TabletResourceManager
trm, Configuration conf,
@@ -1356,9 +1356,9 @@ public class Tablet {
       }
 
     });
-    
+
     acuTableConf.getNamespaceConfiguration().addObserver(configObserver);
-    
+
     // Force a load of any per-table properties
     configObserver.propertiesChanged();
 
@@ -1706,7 +1706,7 @@ public class Tablet {
 
   /**
    * Determine if a JVM shutdown is in progress.
-   * 
+   *
    */
   private boolean shutdownInProgress() {
     try {
@@ -2044,7 +2044,7 @@ public class Tablet {
     public void setInterruptFlag(AtomicBoolean flag) {
       throw new UnsupportedOperationException();
     }
-    
+
   }
 
   private DataFileValue minorCompact(Configuration conf, VolumeManager fs, InMemoryMap memTable,
FileRef tmpDatafile, FileRef newDatafile, FileRef mergeFile,
@@ -2768,7 +2768,7 @@ public class Tablet {
 
   /**
    * Returns a Path object representing the tablet's location on the DFS.
-   * 
+   *
    * @return location
    */
   public Path getLocation() {
@@ -2863,7 +2863,7 @@ public class Tablet {
 
   /**
    * Returns true if a major compaction should be performed on the tablet.
-   * 
+   *
    */
   public boolean needsMajorCompaction(MajorCompactionReason reason) {
     if (majorCompactionInProgress)
@@ -2875,7 +2875,7 @@ public class Tablet {
 
   /**
    * Returns an int representing the total block size of the mapfiles served by this tablet.
-   * 
+   *
    * @return size
    */
   // this is the size of just the mapfiles
@@ -3046,7 +3046,7 @@ public class Tablet {
 
   /**
    * Returns true if this tablet needs to be split
-   * 
+   *
    */
   public synchronized boolean needsSplit() {
     boolean ret;
@@ -3304,7 +3304,7 @@ public class Tablet {
 
     return smallestFiles;
   }
-  
+
   static void rename(VolumeManager fs, Path src, Path dst) throws IOException {
     if (!fs.rename(src, dst)) {
       throw new IOException("Rename " + src + " to " + dst + " returned false ");
@@ -3373,7 +3373,7 @@ public class Tablet {
 
   /**
    * Returns a KeyExtent object representing this tablet's key range.
-   * 
+   *
    * @return extent
    */
   public KeyExtent getExtent() {
@@ -3433,12 +3433,12 @@ public class Tablet {
 
   /**
    * operations are disallowed while we split which is ok since splitting is fast
-   * 
+   *
    * a minor compaction should have taken place before calling this so there should be relatively
little left to compact
-   * 
+   *
    * we just need to make sure major compactions aren't occurring if we have the major compactor
thread decide who needs splitting we can avoid synchronization
    * issues with major compactions
-   * 
+   *
    */
 
   static class SplitInfo {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/6ad16a9c/test/src/test/java/org/apache/accumulo/test/ImportExportIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/ImportExportIT.java b/test/src/test/java/org/apache/accumulo/test/ImportExportIT.java
index c7e0f0e..a48ed9d 100644
--- a/test/src/test/java/org/apache/accumulo/test/ImportExportIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/ImportExportIT.java
@@ -42,7 +42,6 @@ import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.Iterables;
 import com.google.common.io.Files;
 
 /**
@@ -57,8 +56,13 @@ public class ImportExportIT extends SimpleMacIT {
 
   private static final Logger log = LoggerFactory.getLogger(ImportExportIT.class);
 
+  @Override
+  protected int defaultTimeoutSeconds() {
+    return 60;
+  }
+
   @Test
-  public void test() throws Exception {
+  public void testExportImportThenScan() throws Exception {
     Connector conn = getConnector();
 
     String[] tableNames = getUniqueNames(2);
@@ -128,19 +132,34 @@ public class ImportExportIT extends SimpleMacIT {
     Scanner s = conn.createScanner(MetadataTable.NAME, Authorizations.EMPTY);
     s.setRange(MetadataSchema.TabletsSection.getRange(tableId));
     s.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME);
+    MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN.fetch(s);
 
     // Should find a single entry
-    Entry<Key,Value> fileEntry = Iterables.getOnlyElement(s);
-
-    // The file should be an absolute URI (file:///...), not a relative path (/b-000.../I000001.rf)
-    String fileUri = fileEntry.getKey().getColumnQualifier().toString();
-    Assert.assertFalse("Imported files should have absolute URIs, not relative: " + fileUri,
looksLikeRelativePath(fileUri));
+    for (Entry<Key,Value> fileEntry : s) {
+      Key k = fileEntry.getKey();
+      String value = fileEntry.getValue().toString();
+      if (k.getColumnFamily().equals(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME))
{
+        // The file should be an absolute URI (file:///...), not a relative path (/b-000.../I000001.rf)
+        String fileUri = k.getColumnQualifier().toString();
+        Assert.assertFalse("Imported files should have absolute URIs, not relative: " + fileUri,
looksLikeRelativePath(fileUri));
+      } else if (k.getColumnFamily().equals(MetadataSchema.TabletsSection.ServerColumnFamily.NAME))
{
+        Assert.assertFalse("Server directory should have absolute URI, not relative: " +
value, looksLikeRelativePath(value));
+      } else {
+        Assert.fail("Got expected pair: " + k + "=" + fileEntry.getValue());
+      }
+    }
 
     // Online the original table before we verify equivalence
     conn.tableOperations().online(srcTable, true);
 
+    verifyTableEquality(conn, srcTable, destTable);
+  }
+
+  private void verifyTableEquality(Connector conn, String srcTable, String destTable) throws
Exception {
     Iterator<Entry<Key,Value>> src = conn.createScanner(srcTable, Authorizations.EMPTY).iterator(),
dest = conn.createScanner(destTable, Authorizations.EMPTY)
         .iterator();
+    Assert.assertTrue("Could not read any data from source table", src.hasNext());
+    Assert.assertTrue("Could not read any data from destination table", dest.hasNext());
     while (src.hasNext() && dest.hasNext()) {
       Entry<Key,Value> orig = src.next(), copy = dest.next();
       Assert.assertEquals(orig.getKey(), copy.getKey());
@@ -155,6 +174,8 @@ public class ImportExportIT extends SimpleMacIT {
       if ('/' == uri.charAt(10)) {
         return true;
       }
+    } else if (uri.startsWith("/" + Constants.CLONE_PREFIX)) {
+      return true;
     }
 
     return false;


Mime
View raw message