From commits-return-23591-archive-asf-public=cust-asf.ponee.io@accumulo.apache.org Fri Dec 20 14:33:33 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 7B0B618064C for ; Fri, 20 Dec 2019 15:33:33 +0100 (CET) Received: (qmail 96089 invoked by uid 500); 20 Dec 2019 14:33:32 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 96080 invoked by uid 99); 20 Dec 2019 14:33:32 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 20 Dec 2019 14:33:32 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 572D68D80D; Fri, 20 Dec 2019 14:33:32 +0000 (UTC) Date: Fri, 20 Dec 2019 14:33:32 +0000 To: "commits@accumulo.apache.org" Subject: [accumulo] branch master updated: Add code to replace relative paths on upgrade. #1397 (#1461) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <157685241222.13502.11113135192137663299@gitbox.apache.org> From: mmiller@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: accumulo X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 01506c6dd313a6bca42411f12f92839da29ac1d1 X-Git-Newrev: ad107377bf2f2e4751b0b8ebe6c2942a3f078de8 X-Git-Rev: ad107377bf2f2e4751b0b8ebe6c2942a3f078de8 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. mmiller pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/accumulo.git The following commit(s) were added to refs/heads/master by this push: new ad10737 Add code to replace relative paths on upgrade. #1397 (#1461) ad10737 is described below commit ad107377bf2f2e4751b0b8ebe6c2942a3f078de8 Author: Mike Miller AuthorDate: Fri Dec 20 09:33:21 2019 -0500 Add code to replace relative paths on upgrade. #1397 (#1461) * Resolve and replace all relative tablet file paths found in metadata tables with absolute paths during upgrade. Absolute paths are resolved by prefixing relative paths with a volume configured by the user in a the instance.volumes.upgrade.relative property, which is only used during an upgrade. If any relative paths are found and this property is not configured, or if any resolved absolute path does not correspond to a file that actually exists, the upgrade step fails and aborts without making changes. --- .../org/apache/accumulo/core/conf/Property.java | 4 + .../accumulo/master/upgrade/Upgrader9to10.java | 126 +++++++++++ .../accumulo/master/upgrade/Upgrader9to10Test.java | 241 +++++++++++++++++++++ 3 files changed, 371 insertions(+) diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index 34a06a2..6c60524 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -124,6 +124,10 @@ public enum Property { + "currently in use, run 'accumulo admin volumes -l'. To use a comma or " + "other reserved characters in a URI use standard URI hex encoding. For " + "example replace commas with %2C."), + INSTANCE_VOLUMES_UPGRADE_RELATIVE("instance.volumes.upgrade.relative", "", PropertyType.STRING, + "The volume dfs uri containing relative tablet file paths. Relative paths may exist in the metadata from " + + "versions prior to 1.6. This property is only required if a relative path is detected " + + "during the upgrade process and will only be used once."), INSTANCE_SECURITY_AUTHENTICATOR("instance.security.authenticator", "org.apache.accumulo.server.security.handler.ZKAuthenticator", PropertyType.CLASSNAME, "The authenticator class that accumulo will use to determine if a user " diff --git a/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java b/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java index 3075545..68c7db8 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java +++ b/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java @@ -44,11 +44,13 @@ import org.apache.accumulo.core.client.MutationsRejectedException; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.client.admin.TimeType; +import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Mutation; import org.apache.accumulo.core.data.Range; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.file.FileOperations; import org.apache.accumulo.core.file.FileSKVIterator; import org.apache.accumulo.core.metadata.RootTable; @@ -65,6 +67,7 @@ import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.fate.zookeeper.ZooUtil; import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeMissingPolicy; +import org.apache.accumulo.server.ServerConstants; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.fs.FileRef; import org.apache.accumulo.server.fs.VolumeManager; @@ -110,12 +113,14 @@ public class Upgrader9to10 implements Upgrader { @Override public void upgradeRoot(ServerContext ctx) { + upgradeRelativePaths(ctx, Ample.DataLevel.METADATA); upgradeDirColumns(ctx, Ample.DataLevel.METADATA); upgradeFileDeletes(ctx, Ample.DataLevel.METADATA); } @Override public void upgradeMetadata(ServerContext ctx) { + upgradeRelativePaths(ctx, Ample.DataLevel.USER); upgradeDirColumns(ctx, Ample.DataLevel.USER); upgradeFileDeletes(ctx, Ample.DataLevel.USER); } @@ -500,4 +505,125 @@ public class Upgrader9to10 implements Upgrader { public static String upgradeDirColumn(String dir) { return new Path(dir).getName(); } + + /** + * Remove all file entries containing relative paths and replace them with absolute URI paths. + */ + public static void upgradeRelativePaths(ServerContext ctx, Ample.DataLevel level) { + String tableName = level.metaTable(); + AccumuloClient c = ctx; + VolumeManager fs = ctx.getVolumeManager(); + String upgradeProp = ctx.getConfiguration().get(Property.INSTANCE_VOLUMES_UPGRADE_RELATIVE); + + // first pass check for relative paths - if any, check existence of the file path + // constructed from the upgrade property + relative path + if (checkForRelativePaths(c, fs, tableName, upgradeProp)) { + log.info("Relative Tablet File paths exist in {}, replacing with absolute using {}", + tableName, upgradeProp); + } else { + log.info("No relative paths found in {} during upgrade.", tableName); + return; + } + + // second pass, create atomic mutations to replace the relative path + replaceRelativePaths(c, fs, tableName, upgradeProp); + } + + /** + * Replace relative paths but only if the constructed absolute path exists on FileSystem + */ + public static void replaceRelativePaths(AccumuloClient c, VolumeManager fs, String tableName, + String upgradeProperty) { + try (Scanner scanner = c.createScanner(tableName, Authorizations.EMPTY); + BatchWriter writer = c.createBatchWriter(tableName)) { + + scanner.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); + for (Entry entry : scanner) { + Key key = entry.getKey(); + String metaEntry = key.getColumnQualifier().toString(); + if (!metaEntry.contains(":")) { + // found relative paths so get the property used to build the absolute paths + if (upgradeProperty == null || upgradeProperty.isBlank()) { + throw new IllegalArgumentException( + "Missing required property " + Property.INSTANCE_VOLUMES_UPGRADE_RELATIVE.getKey()); + } + Path relPath = resolveRelativePath(metaEntry, key); + Path absPath = new Path(upgradeProperty, relPath); + // Path absPath = new Path(Paths.get(upgradeProperty).normalize().toString(), relPath); + if (fs.exists(absPath)) { + log.debug("Changing Tablet File path from {} to {}", metaEntry, absPath); + Mutation m = new Mutation(key.getRow()); + // add the new path + m.at().family(key.getColumnFamily()).qualifier(absPath.toString()) + .visibility(key.getColumnVisibility()).put(entry.getValue()); + // delete the old path + m.at().family(key.getColumnFamily()).qualifier(key.getColumnQualifierData().toArray()) + .visibility(key.getColumnVisibility()).delete(); + writer.addMutation(m); + } else { + throw new IllegalArgumentException( + "Relative Tablet file " + relPath + " not found at " + absPath); + } + } + } + } catch (MutationsRejectedException | TableNotFoundException e) { + throw new IllegalStateException(e); + } catch (IOException ioe) { + throw new UncheckedIOException(ioe); + } + } + + /** + * Check if table has any relative paths, return false if none are found. When a relative path is + * found, check existence of the file path constructed from the upgrade property + relative path + */ + public static boolean checkForRelativePaths(AccumuloClient client, VolumeManager fs, + String tableName, String upgradeProperty) { + boolean hasRelatives = false; + + try (Scanner scanner = client.createScanner(tableName, Authorizations.EMPTY)) { + log.info("Looking for relative paths in {}", tableName); + scanner.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); + for (Entry entry : scanner) { + Key key = entry.getKey(); + String metaEntry = key.getColumnQualifier().toString(); + if (!metaEntry.contains(":")) { + // found relative paths so verify the property used to build the absolute paths + hasRelatives = true; + if (upgradeProperty == null || upgradeProperty.isBlank()) { + throw new IllegalArgumentException( + "Missing required property " + Property.INSTANCE_VOLUMES_UPGRADE_RELATIVE.getKey()); + } + Path relPath = resolveRelativePath(metaEntry, key); + Path absPath = new Path(upgradeProperty, relPath); + // Path absPath = new Path(Paths.get(upgradeProperty).normalize().toString(), relPath); + if (!fs.exists(absPath)) { + throw new IllegalArgumentException("Tablet file " + relPath + " not found at " + absPath + + " using volume: " + upgradeProperty); + } + } + } + } catch (TableNotFoundException e) { + throw new IllegalStateException(e); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + + return hasRelatives; + } + + /** + * Resolve old-style relative paths, returning Path of everything except volume and base + */ + private static Path resolveRelativePath(String metadataEntry, Key key) { + String prefix = ServerConstants.TABLE_DIR + "/"; + if (metadataEntry.startsWith("../")) { + // resolve style "../2a/t-0003/C0004.rf" + return new Path(prefix + metadataEntry.substring(3)); + } else { + // resolve style "/t-0003/C0004.rf" + TableId tableId = KeyExtent.tableOfMetadataRow(key.getRow()); + return new Path(prefix + tableId.canonical() + metadataEntry); + } + } } diff --git a/server/master/src/test/java/org/apache/accumulo/master/upgrade/Upgrader9to10Test.java b/server/master/src/test/java/org/apache/accumulo/master/upgrade/Upgrader9to10Test.java index df0b3ab..9b78dad 100644 --- a/server/master/src/test/java/org/apache/accumulo/master/upgrade/Upgrader9to10Test.java +++ b/server/master/src/test/java/org/apache/accumulo/master/upgrade/Upgrader9to10Test.java @@ -19,13 +19,44 @@ package org.apache.accumulo.master.upgrade; import static org.apache.accumulo.core.Constants.BULK_PREFIX; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.SortedMap; +import java.util.TreeMap; + +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.BatchWriter; +import org.apache.accumulo.core.client.MutationsRejectedException; +import org.apache.accumulo.core.client.Scanner; +import org.apache.accumulo.core.data.ColumnUpdate; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Mutation; import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.metadata.schema.Ample; +import org.apache.accumulo.core.metadata.schema.MetadataSchema; +import org.apache.accumulo.core.security.Authorizations; +import org.apache.accumulo.server.fs.VolumeManager; import org.apache.accumulo.server.gc.GcVolumeUtil; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.io.Text; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class Upgrader9to10Test { + private static Logger log = LoggerFactory.getLogger(Upgrader9to10Test.class); + @Test public void testSwitchRelative() { assertEquals(GcVolumeUtil.getDeleteTabletOnAllVolumesUri(TableId.of("5a"), "t-0005"), @@ -63,4 +94,214 @@ public class Upgrader9to10Test { assertEquals("t-0005", Upgrader9to10.upgradeDirColumn("/t-0005")); assertEquals("t-0005", Upgrader9to10.upgradeDirColumn("t-0005")); } + + String tableName = Ample.DataLevel.USER.metaTable(); + String volumeUpgrade = "file:///accumulo"; + + // mock objects for testing relative path replacement + private void setupMocks(AccumuloClient c, VolumeManager fs, SortedMap map, + List results) throws Exception { + Scanner scanner = createMock(Scanner.class); + // buffer all the mutations that are created so we can verify they are correct + BatchWriter writer = new BatchWriter() { + List buffer = new ArrayList<>(); + + @Override + public void addMutation(Mutation m) throws MutationsRejectedException { + buffer.add(m); + } + + @Override + public void addMutations(Iterable iterable) throws MutationsRejectedException { + iterable.forEach(buffer::add); + } + + @Override + public void flush() throws MutationsRejectedException {} + + @Override + // simulate the close by adding all to results and preventing anymore adds + public void close() throws MutationsRejectedException { + results.addAll(buffer); + buffer = null; + } + }; + + expect(c.createScanner(tableName, Authorizations.EMPTY)).andReturn(scanner).anyTimes(); + expect(c.createBatchWriter(tableName)).andReturn(writer).anyTimes(); + expect(scanner.iterator()).andReturn(map.entrySet().iterator()).anyTimes(); + + // void methods + scanner.fetchColumnFamily(MetadataSchema.TabletsSection.DataFileColumnFamily.NAME); + expectLastCall().anyTimes(); + scanner.close(); + expectLastCall().anyTimes(); + + replay(c, fs, scanner); + } + + @Test + public void noRelativePaths() throws Exception { + VolumeManager fs = createMock(VolumeManager.class); + AccumuloClient c = createMock(AccumuloClient.class); + expect(fs.exists(anyObject())).andReturn(true).anyTimes(); + + SortedMap map = new TreeMap<>(); + map.put(new Key("1<", "file", "hdfs://nn1:8020/accumulo/tables/1/default_tablet/A000001c.rf"), + new Value()); + map.put(new Key("1<", "file", "hdfs://nn1:8020/accumulo/tables/1/default_tablet/F000001m.rf"), + new Value()); + map.put(new Key("1<", "file", "file://nn1:8020/accumulo/tables/1/t-0005/F000004x.rf"), + new Value()); + map.put(new Key("1<", "file", "file://volume23:8000/accumulo/tables/1/t-1234/F0000054.rf"), + new Value()); + + List results = new ArrayList<>(); + + setupMocks(c, fs, map, results); + assertEquals("Invalid Relative path check", + Upgrader9to10.checkForRelativePaths(c, fs, tableName, volumeUpgrade), false); + assertTrue(results.isEmpty()); + } + + @Test + public void filesDontExistAfterReplacingRelatives() throws Exception { + AccumuloClient c = createMock(AccumuloClient.class); + VolumeManager fs = createMock(VolumeManager.class); + SortedMap map = new TreeMap<>(); + map.put(new Key("1b;row_000050", "file", "../1b/default_tablet/A000001c.rf"), new Value("1")); + map.put(new Key("1b;row_000050", "file", "../1b/default_tablet/F000001m.rf"), new Value("2")); + + expect(fs.exists(anyObject(Path.class))).andReturn(false).anyTimes(); + + setupMocks(c, fs, map, new ArrayList<>()); + try { + Upgrader9to10.checkForRelativePaths(c, fs, tableName, volumeUpgrade); + fail("Expected IllegalArgumentException to be thrown"); + } catch (IllegalArgumentException e) {} + } + + @Test + public void missingUpgradeRelativeProperty() throws Exception { + AccumuloClient c = createMock(AccumuloClient.class); + VolumeManager fs = createMock(VolumeManager.class); + SortedMap map = new TreeMap<>(); + map.put(new Key("1b;row_000050", "file", "../1b/default_tablet/A000001c.rf"), new Value("1")); + map.put(new Key("1b;row_000050", "file", "../1b/default_tablet/F000001m.rf"), new Value("2")); + + expect(fs.exists(anyObject(Path.class))).andReturn(false).anyTimes(); + + setupMocks(c, fs, map, new ArrayList<>()); + try { + Upgrader9to10.checkForRelativePaths(c, fs, tableName, ""); + fail("Expected IllegalArgumentException to be thrown"); + } catch (IllegalArgumentException e) {} + } + + @Test + public void replaceRelatives() throws Exception { + AccumuloClient c = createMock(AccumuloClient.class); + VolumeManager fs = createMock(VolumeManager.class); + expect(fs.exists(anyObject())).andReturn(true).anyTimes(); + + SortedMap map = new TreeMap<>(); + map.put(new Key("1b;row_000050", "file", "../1b/default_tablet/A000001c.rf"), new Value("1")); + map.put(new Key("1b;row_000050", "file", "../1b/default_tablet/F000001m.rf"), new Value("2")); + map.put(new Key("1b;row_000050", "file", "../1b/t-000008t/F000004x.rf"), new Value("3")); + map.put(new Key("1b;row_000050", "file", "/t-000008t/F0000054.rf"), new Value("4")); + map.put(new Key("1b<", "file", "../1b/default_tablet/A000001c.rf"), new Value("1")); + map.put(new Key("1b<", "file", "../1b/default_tablet/F000001m.rf"), new Value("2")); + map.put(new Key("1b<", "file", "../1b/t-000008t/F000004x.rf"), new Value("3")); + map.put(new Key("1b<", "file", "/t-000008t/F0000054.rf"), new Value("4")); + map.put(new Key("1b<", "file", "hdfs://nn1:8020/accumulo/tables/1b/t-000008t/A0000098.rf"), + new Value("5")); + map.put(new Key("1b<", "file", "hdfs://nn1:8020/accumulo/tables/1b/t-000008t/F0000098.rf"), + new Value("5")); + + List expected = new ArrayList<>(); + expected.add(replaceMut("1b;row_000050", "file:/accumulo/tables/1b/default_tablet/A000001c.rf", + "1", "../1b/default_tablet/A000001c.rf")); + expected.add(replaceMut("1b;row_000050", "file:/accumulo/tables/1b/default_tablet/F000001m.rf", + "2", "../1b/default_tablet/F000001m.rf")); + expected.add(replaceMut("1b;row_000050", "file:/accumulo/tables/1b/t-000008t/F000004x.rf", "3", + "../1b/t-000008t/F000004x.rf")); + expected.add(replaceMut("1b;row_000050", "file:/accumulo/tables/1b/t-000008t/F0000054.rf", "4", + "/t-000008t/F0000054.rf")); + expected.add(replaceMut("1b<", "file:/accumulo/tables/1b/default_tablet/A000001c.rf", "1", + "../1b/default_tablet/A000001c.rf")); + expected.add(replaceMut("1b<", "file:/accumulo/tables/1b/default_tablet/F000001m.rf", "2", + "../1b/default_tablet/F000001m.rf")); + expected.add(replaceMut("1b<", "file:/accumulo/tables/1b/t-000008t/F000004x.rf", "3", + "../1b/t-000008t/F000004x.rf")); + expected.add(replaceMut("1b<", "file:/accumulo/tables/1b/t-000008t/F0000054.rf", "4", + "/t-000008t/F0000054.rf")); + + List results = new ArrayList<>(); + + setupMocks(c, fs, map, results); + Upgrader9to10.replaceRelativePaths(c, fs, tableName, volumeUpgrade); + verifyPathsReplaced(expected, results); + } + + @Test + public void normalizeVolume() throws Exception { + String uglyVolume = "hdfs://nn.somewhere.com:86753/accumulo/blah/.././/bad/bad2/../.././/////"; + + AccumuloClient c = createMock(AccumuloClient.class); + VolumeManager fs = createMock(VolumeManager.class); + expect(fs.exists(anyObject())).andReturn(true).anyTimes(); + SortedMap map = new TreeMap<>(); + map.put(new Key("1b<", "file", "../1b/t-000008t/F000004x.rf"), new Value("1")); + map.put(new Key("1b<", "file", "/t-000008t/F0000054.rf"), new Value("2")); + List results = new ArrayList<>(); + List expected = new ArrayList<>(); + expected.add( + replaceMut("1b<", "hdfs://nn.somewhere.com:86753/accumulo/tables/1b/t-000008t/F000004x.rf", + "1", "../1b/t-000008t/F000004x.rf")); + expected.add( + replaceMut("1b<", "hdfs://nn.somewhere.com:86753/accumulo/tables/1b/t-000008t/F0000054.rf", + "2", "/t-000008t/F0000054.rf")); + + setupMocks(c, fs, map, results); + Upgrader9to10.replaceRelativePaths(c, fs, tableName, uglyVolume); + verifyPathsReplaced(expected, results); + } + + private Mutation replaceMut(String row, String cq, String val, String delete) { + Mutation m = new Mutation(row); + m.at().family("file").qualifier(cq).put(new Value(val)); + m.at().family("file").qualifier(delete).delete(); + return m; + } + + /** + * Make sure mutations are all the same, in the correct order + */ + private void verifyPathsReplaced(List expected, List results) { + Iterator expectIter = expected.iterator(); + int deleteCount = 0; + int updateCount = 0; + for (Mutation mut : results) { + Mutation next = expectIter.next(); + Iterator nextUpdates = next.getUpdates().iterator(); + assertEquals(next.getUpdates().size(), mut.getUpdates().size()); + assertEquals(new Text(next.getRow()), new Text(mut.getRow())); + + // check updates are all the same + for (ColumnUpdate update : mut.getUpdates()) { + ColumnUpdate nextUpdate = nextUpdates.next(); + Text cq = new Text(nextUpdate.getColumnQualifier()); + log.debug("Checking for expected columnUpdate: " + cq + " deleted? " + update.isDeleted()); + assertEquals(cq, new Text(update.getColumnQualifier())); + if (update.isDeleted()) { + deleteCount++; + } else { + updateCount++; + assertEquals(new Text(nextUpdate.getValue()), new Text(update.getValue())); + } + } + } + + assertEquals("Replacements should have update for every delete", deleteCount, updateCount); + } }