Return-Path: X-Original-To: apmail-accumulo-commits-archive@www.apache.org Delivered-To: apmail-accumulo-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 27C99D6EC for ; Fri, 12 Oct 2012 00:06:43 +0000 (UTC) Received: (qmail 1183 invoked by uid 500); 12 Oct 2012 00:06:43 -0000 Delivered-To: apmail-accumulo-commits-archive@accumulo.apache.org Received: (qmail 1156 invoked by uid 500); 12 Oct 2012 00:06:43 -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 1147 invoked by uid 99); 12 Oct 2012 00:06:43 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 12 Oct 2012 00:06:43 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 12 Oct 2012 00:06:41 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 0DFB523889C5 for ; Fri, 12 Oct 2012 00:05:58 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1397383 - in /accumulo/branches/1.4/src/core/src: main/java/org/apache/accumulo/core/client/impl/ main/java/org/apache/accumulo/core/util/ test/java/org/apache/accumulo/core/client/impl/ Date: Fri, 12 Oct 2012 00:05:57 -0000 To: commits@accumulo.apache.org From: kturner@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20121012000558.0DFB523889C5@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: kturner Date: Fri Oct 12 00:05:56 2012 New Revision: 1397383 URL: http://svn.apache.org/viewvc?rev=1397383&view=rev Log: ACCUMULO-806 fixed bug where a tablet location could not be found when the metadata table had empty tablets Modified: accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/MetadataLocationObtainer.java accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/MetadataTable.java accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java Modified: accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/MetadataLocationObtainer.java URL: http://svn.apache.org/viewvc/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/MetadataLocationObtainer.java?rev=1397383&r1=1397382&r2=1397383&view=diff ============================================================================== --- accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/MetadataLocationObtainer.java (original) +++ accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/MetadataLocationObtainer.java Fri Oct 12 00:05:56 2012 @@ -32,6 +32,7 @@ import org.apache.accumulo.core.client.A import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.Instance; import org.apache.accumulo.core.client.impl.TabletLocator.TabletLocation; +import org.apache.accumulo.core.client.impl.TabletLocator.TabletLocations; import org.apache.accumulo.core.client.impl.TabletLocatorImpl.TabletLocationObtainer; import org.apache.accumulo.core.client.impl.TabletServerBatchReaderIterator.ResultReceiver; import org.apache.accumulo.core.data.Column; @@ -44,6 +45,7 @@ import org.apache.accumulo.core.security import org.apache.accumulo.core.tabletserver.thrift.NotServingTabletException; import org.apache.accumulo.core.util.MetadataTable; import org.apache.accumulo.core.util.OpTimer; +import org.apache.accumulo.core.util.Pair; import org.apache.accumulo.core.util.TextUtil; import org.apache.hadoop.io.Text; import org.apache.log4j.Level; @@ -68,12 +70,12 @@ public class MetadataLocationObtainer im } @Override - public List lookupTablet(TabletLocation src, Text row, Text stopRow, TabletLocator parent) throws AccumuloSecurityException, + public TabletLocations lookupTablet(TabletLocation src, Text row, Text stopRow, TabletLocator parent) throws AccumuloSecurityException, AccumuloException { - - ArrayList list = new ArrayList(); - + try { + ArrayList list = new ArrayList(); + OpTimer opTimer = null; if (log.isTraceEnabled()) opTimer = new OpTimer(log, Level.TRACE).start("Looking up in " + src.tablet_extent.getTableId() + " row=" + TextUtil.truncate(row) + " extent=" @@ -98,12 +100,14 @@ public class MetadataLocationObtainer im // System.out.println("results "+results.keySet()); - SortedMap metadata = MetadataTable.getMetadataLocationEntries(results); + Pair,List> metadata = MetadataTable.getMetadataLocationEntries(results); - for (Entry entry : metadata.entrySet()) { + for (Entry entry : metadata.getFirst().entrySet()) { list.add(new TabletLocation(entry.getKey(), entry.getValue().toString())); } + return new TabletLocations(list, metadata.getSecond()); + } catch (AccumuloServerException ase) { if (log.isTraceEnabled()) log.trace(src.tablet_extent.getTableId() + " lookup failed, " + src.tablet_location + " server side exception"); @@ -118,7 +122,7 @@ public class MetadataLocationObtainer im parent.invalidateCache(src.tablet_location); } - return list; + return null; } @Override @@ -159,7 +163,7 @@ public class MetadataLocationObtainer im throw e; } - SortedMap metadata = MetadataTable.getMetadataLocationEntries(results); + SortedMap metadata = MetadataTable.getMetadataLocationEntries(results).getFirst(); for (Entry entry : metadata.entrySet()) { list.add(new TabletLocation(entry.getKey(), entry.getValue().toString())); Modified: accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java URL: http://svn.apache.org/viewvc/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java?rev=1397383&r1=1397382&r2=1397383&view=diff ============================================================================== --- accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java (original) +++ accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocator.java Fri Oct 12 00:05:56 2012 @@ -126,6 +126,25 @@ public abstract class TabletLocator { return tl; } + public static class TabletLocations { + + private final List locations; + private final List locationless; + + public TabletLocations(List locations, List locationless) { + this.locations = locations; + this.locationless = locationless; + } + + public List getLocations() { + return locations; + } + + public List getLocationless() { + return locationless; + } + } + public static class TabletLocation implements Comparable { private static WeakHashMap> tabletLocs = new WeakHashMap>(); Modified: accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java URL: http://svn.apache.org/viewvc/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java?rev=1397383&r1=1397382&r2=1397383&view=diff ============================================================================== --- accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java (original) +++ accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java Fri Oct 12 00:05:56 2012 @@ -92,7 +92,10 @@ public class TabletLocatorImpl extends T private Lock wLock = rwLock.writeLock(); public static interface TabletLocationObtainer { - List lookupTablet(TabletLocation src, Text row, Text stopRow, TabletLocator parent) throws AccumuloSecurityException, AccumuloException; + /** + * @return null when unable to read information successfully + */ + TabletLocations lookupTablet(TabletLocation src, Text row, Text stopRow, TabletLocator parent) throws AccumuloSecurityException, AccumuloException; List lookupTablets(String tserver, Map> map, TabletLocator parent) throws AccumuloSecurityException, AccumuloException; @@ -390,23 +393,31 @@ public class TabletLocatorImpl extends T TabletLocation ptl = parent.locateTablet(metadataRow, false, retry); if (ptl != null) { - List locations = locationObtainer.lookupTablet(ptl, metadataRow, lastTabletRow, parent); - if (locations.size() == 0 && !ptl.tablet_extent.equals(Constants.ROOT_TABLET_EXTENT)) { - // try the next tablet + TabletLocations locations = locationObtainer.lookupTablet(ptl, metadataRow, lastTabletRow, parent); + while (locations != null && locations.getLocations().isEmpty() && locations.getLocationless().isEmpty() + && !ptl.tablet_extent.equals(Constants.ROOT_TABLET_EXTENT)) { + // try the next tablet, the current tablet does not have any tablets that overlap the row Text er = ptl.tablet_extent.getEndRow(); if (er != null && er.compareTo(lastTabletRow) < 0) { // System.out.println("er "+er+" ltr "+lastTabletRow); ptl = parent.locateTablet(er, true, retry); if (ptl != null) locations = locationObtainer.lookupTablet(ptl, metadataRow, lastTabletRow, parent); + else + break; + } else { + break; } } + if (locations == null) + return; + // cannot assume the list contains contiguous key extents... so it is probably // best to deal with each extent individually Text lastEndRow = null; - for (TabletLocation tabletLocation : locations) { + for (TabletLocation tabletLocation : locations.getLocations()) { KeyExtent ke = tabletLocation.tablet_extent; TabletLocation locToCache; Modified: accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/MetadataTable.java URL: http://svn.apache.org/viewvc/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/MetadataTable.java?rev=1397383&r1=1397382&r2=1397383&view=diff ============================================================================== --- accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/MetadataTable.java (original) +++ accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/MetadataTable.java Fri Oct 12 00:05:56 2012 @@ -16,6 +16,7 @@ */ package org.apache.accumulo.core.util; +import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -118,7 +119,7 @@ public class MetadataTable { } } - public static SortedMap getMetadataLocationEntries(SortedMap entries) { + public static Pair,List> getMetadataLocationEntries(SortedMap entries) { Key key; Value val; Text location = null; @@ -126,6 +127,7 @@ public class MetadataTable { KeyExtent ke; SortedMap results = new TreeMap(); + ArrayList locationless = new ArrayList(); Text lastRowFromKey = new Text(); @@ -152,15 +154,19 @@ public class MetadataTable { else if (Constants.METADATA_PREV_ROW_COLUMN.equals(colf, colq)) prevRow = new Value(val); - if (location != null && prevRow != null) { + if (prevRow != null) { ke = new KeyExtent(key.getRow(), prevRow); - results.put(ke, location); - + if (location != null) + results.put(ke, location); + else + locationless.add(ke); + location = null; prevRow = null; } } - return results; + + return new Pair,List>(results, locationless); } public static SortedMap> getTabletEntries(Instance instance, KeyExtent ke, List columns, AuthInfo credentials) { Modified: accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java URL: http://svn.apache.org/viewvc/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java?rev=1397383&r1=1397382&r2=1397383&view=diff ============================================================================== --- accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java (original) +++ accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java Fri Oct 12 00:05:56 2012 @@ -37,6 +37,7 @@ import org.apache.accumulo.core.client.A import org.apache.accumulo.core.client.Connector; import org.apache.accumulo.core.client.Instance; import org.apache.accumulo.core.client.impl.TabletLocator.TabletLocation; +import org.apache.accumulo.core.client.impl.TabletLocator.TabletLocations; import org.apache.accumulo.core.client.impl.TabletLocator.TabletServerMutations; import org.apache.accumulo.core.client.impl.TabletLocatorImpl.TabletLocationObtainer; import org.apache.accumulo.core.conf.AccumuloConfiguration; @@ -48,6 +49,7 @@ import org.apache.accumulo.core.data.Ran import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.security.thrift.AuthInfo; import org.apache.accumulo.core.util.MetadataTable; +import org.apache.accumulo.core.util.Pair; import org.apache.hadoop.io.Text; public class TabletLocatorImplTest extends TestCase { @@ -461,7 +463,7 @@ public class TabletLocatorImplTest exten } @Override - public List lookupTablet(TabletLocation src, Text row, Text stopRow, TabletLocator parent) throws AccumuloSecurityException { + public TabletLocations lookupTablet(TabletLocation src, Text row, Text stopRow, TabletLocator parent) throws AccumuloSecurityException { // System.out.println("lookupTablet("+src+","+row+","+stopRow+","+ parent+")"); // System.out.println(tservers); @@ -472,14 +474,14 @@ public class TabletLocatorImplTest exten if (tablets == null) { parent.invalidateCache(src.tablet_location); - return list; + return null; } SortedMap tabletData = tablets.get(src.tablet_extent); if (tabletData == null) { parent.invalidateCache(src.tablet_extent); - return list; + return null; } // the following clip is done on a tablet, do it here to see if it throws exceptions @@ -490,13 +492,13 @@ public class TabletLocatorImplTest exten SortedMap results = tabletData.tailMap(startKey).headMap(stopKey); - SortedMap metadata = MetadataTable.getMetadataLocationEntries(results); + Pair,List> metadata = MetadataTable.getMetadataLocationEntries(results); - for (Entry entry : metadata.entrySet()) { + for (Entry entry : metadata.getFirst().entrySet()) { list.add(new TabletLocation(entry.getKey(), entry.getValue().toString())); } - return list; + return new TabletLocations(list, metadata.getSecond()); } @Override @@ -545,7 +547,7 @@ public class TabletLocatorImplTest exten if (failures.size() > 0) parent.invalidateCache(failures); - SortedMap metadata = MetadataTable.getMetadataLocationEntries(results); + SortedMap metadata = MetadataTable.getMetadataLocationEntries(results).getFirst(); for (Entry entry : metadata.entrySet()) { list.add(new TabletLocation(entry.getKey(), entry.getValue().toString())); @@ -557,6 +559,22 @@ public class TabletLocatorImplTest exten } + static void createEmptyTablet(TServers tservers, String server, KeyExtent tablet) { + Map> tablets = tservers.tservers.get(server); + if (tablets == null) { + tablets = new HashMap>(); + tservers.tservers.put(server, tablets); + } + + SortedMap tabletData = tablets.get(tablet); + if (tabletData == null) { + tabletData = new TreeMap(); + tablets.put(tablet, tabletData); + } else if (tabletData.size() > 0) { + throw new RuntimeException("Asked for empty tablet, but non empty tablet exists"); + } + } + static void setLocation(TServers tservers, String server, KeyExtent tablet, KeyExtent ke, String location) { Map> tablets = tservers.tservers.get(server); if (tablets == null) { @@ -1185,4 +1203,40 @@ public class TabletLocatorImplTest exten assertNull(tab0TabletCache.locateTablet(new Text("row_0000000000"), false, false)); } + + // this test reproduces a problem where empty metadata tablets, that were created by user tablets being merged away, caused locating tablets to fail + public void testBug3() throws Exception { + + KeyExtent mte1 = new KeyExtent(new Text(Constants.METADATA_TABLE_ID), new Text("1;c"), RTE.getEndRow()); + KeyExtent mte2 = new KeyExtent(new Text(Constants.METADATA_TABLE_ID), new Text("1;f"), new Text("1;c")); + KeyExtent mte3 = new KeyExtent(new Text(Constants.METADATA_TABLE_ID), new Text("1;j"), new Text("1;f")); + KeyExtent mte4 = new KeyExtent(new Text(Constants.METADATA_TABLE_ID), new Text("1;r"), new Text("1;j")); + KeyExtent mte5 = new KeyExtent(new Text(Constants.METADATA_TABLE_ID), null, new Text("1;r")); + + KeyExtent ke1 = new KeyExtent(new Text("1"), null, null); + + TServers tservers = new TServers(); + TestTabletLocationObtainer ttlo = new TestTabletLocationObtainer(tservers); + TestInstance testInstance = new TestInstance("instance1", "tserver1"); + + RootTabletLocator rtl = new RootTabletLocator(testInstance); + + TabletLocatorImpl rootTabletCache = new TabletLocatorImpl(new Text(Constants.METADATA_TABLE_ID), rtl, ttlo); + TabletLocatorImpl tab0TabletCache = new TabletLocatorImpl(new Text("1"), rootTabletCache, ttlo); + + setLocation(tservers, "tserver1", RTE, mte1, "tserver2"); + setLocation(tservers, "tserver1", RTE, mte2, "tserver3"); + setLocation(tservers, "tserver1", RTE, mte3, "tserver4"); + setLocation(tservers, "tserver1", RTE, mte4, "tserver5"); + setLocation(tservers, "tserver1", RTE, mte5, "tserver6"); + + createEmptyTablet(tservers, "tserver2", mte1); + createEmptyTablet(tservers, "tserver3", mte2); + createEmptyTablet(tservers, "tserver4", mte3); + createEmptyTablet(tservers, "tserver5", mte4); + setLocation(tservers, "tserver6", mte5, ke1, "tserver7"); + + locateTabletTest(tab0TabletCache, "a", ke1, "tserver7"); + + } }