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 0AABCF3B1 for ; Mon, 8 Apr 2013 13:32:44 +0000 (UTC) Received: (qmail 29512 invoked by uid 500); 8 Apr 2013 13:32:44 -0000 Delivered-To: apmail-accumulo-commits-archive@accumulo.apache.org Received: (qmail 29441 invoked by uid 500); 8 Apr 2013 13:32: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 29431 invoked by uid 99); 8 Apr 2013 13:32:43 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 08 Apr 2013 13:32: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; Mon, 08 Apr 2013 13:32:39 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 5202E23888FD; Mon, 8 Apr 2013 13:32:17 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1465630 - in /accumulo/trunk: ./ assemble/ core/ examples/ fate/src/main/java/org/apache/accumulo/fate/ fate/src/main/java/org/apache/accumulo/fate/zookeeper/ server/ server/src/main/java/org/apache/accumulo/server/master/state/ server/src... Date: Mon, 08 Apr 2013 13:32:16 -0000 To: commits@accumulo.apache.org From: ecn@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20130408133217.5202E23888FD@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: ecn Date: Mon Apr 8 13:32:16 2013 New Revision: 1465630 URL: http://svn.apache.org/r1465630 Log: ACCUMULO-1247 detect additional bad location states Modified: accumulo/trunk/ (props changed) accumulo/trunk/assemble/ (props changed) accumulo/trunk/core/ (props changed) accumulo/trunk/examples/ (props changed) accumulo/trunk/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java (props changed) accumulo/trunk/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java (props changed) accumulo/trunk/server/ (props changed) accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MergeStats.java accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletStateChangeIterator.java accumulo/trunk/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java accumulo/trunk/server/src/test/java/org/apache/accumulo/server/master/state/RootTabletStateStoreTest.java accumulo/trunk/src/ (props changed) Propchange: accumulo/trunk/ ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5:r1465627 Propchange: accumulo/trunk/assemble/ ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/assemble:r1465627 Propchange: accumulo/trunk/core/ ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/core:r1465627 Propchange: accumulo/trunk/examples/ ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/examples:r1465627 Propchange: accumulo/trunk/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/fate/src/main/java/org/apache/accumulo/fate/ZooStore.java:r1465627 Propchange: accumulo/trunk/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java:r1465627 Propchange: accumulo/trunk/server/ ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/server:r1465627 Modified: accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MergeStats.java URL: http://svn.apache.org/viewvc/accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MergeStats.java?rev=1465630&r1=1465629&r2=1465630&view=diff ============================================================================== --- accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MergeStats.java (original) +++ accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MergeStats.java Mon Apr 8 13:32:16 2013 @@ -22,6 +22,7 @@ import java.util.Map.Entry; import org.apache.accumulo.core.Constants; import org.apache.accumulo.server.cli.ClientOpts; +import org.apache.accumulo.server.master.state.TabletLocationState.BadLocationStateException; import org.apache.accumulo.core.client.Connector; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.client.TableNotFoundException; @@ -187,7 +188,13 @@ public class MergeStats { KeyExtent prevExtent = null; for (Entry entry : scanner) { - TabletLocationState tls = MetaDataTableScanner.createTabletLocationState(entry.getKey(), entry.getValue()); + TabletLocationState tls; + try { + tls = MetaDataTableScanner.createTabletLocationState(entry.getKey(), entry.getValue()); + } catch (BadLocationStateException e) { + log.error(e, e); + return false; + } if (!tls.extent.getTableId().equals(tableId)) { break; } Modified: accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java URL: http://svn.apache.org/viewvc/accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java?rev=1465630&r1=1465629&r2=1465630&view=diff ============================================================================== --- accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java (original) +++ accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java Mon Apr 8 13:32:16 2013 @@ -39,6 +39,7 @@ import org.apache.accumulo.core.data.Val import org.apache.accumulo.core.iterators.user.WholeRowIterator; import org.apache.accumulo.core.security.CredentialHelper; import org.apache.accumulo.core.security.thrift.TCredentials; +import org.apache.accumulo.server.master.state.TabletLocationState.BadLocationStateException; import org.apache.hadoop.io.Text; import org.apache.log4j.Logger; @@ -109,16 +110,13 @@ public class MetaDataTableScanner implem try { return fetch(); } catch (RuntimeException ex) { - try { - close(); - } catch (Exception e) { - log.error(e, e); - } - throw ex; - } + // something is wrong with the records in the !METADATA table, just skip over it + log.error(ex, ex); + return null; + } } - public static TabletLocationState createTabletLocationState(Key k, Value v) throws IOException { + public static TabletLocationState createTabletLocationState(Key k, Value v) throws IOException, BadLocationStateException { final SortedMap decodedRow = WholeRowIterator.decodeRow(k, v); KeyExtent extent = null; TServerInstance future = null; @@ -134,13 +132,25 @@ public class MetaDataTableScanner implem Text cq = key.getColumnQualifier(); if (cf.compareTo(Constants.METADATA_FUTURE_LOCATION_COLUMN_FAMILY) == 0) { - future = new TServerInstance(entry.getValue(), cq); + TServerInstance location = new TServerInstance(entry.getValue(), cq); + if (future != null) { + throw new BadLocationStateException("found two assignments for the same extent " + key.getRow() + ": " + future + " and " + location); + } + future = location; } else if (cf.compareTo(Constants.METADATA_CURRENT_LOCATION_COLUMN_FAMILY) == 0) { - current = new TServerInstance(entry.getValue(), cq); + TServerInstance location = new TServerInstance(entry.getValue(), cq); + if (current != null) { + throw new BadLocationStateException("found two locations for the same extent " + key.getRow() + ": " + current + " and " + location); + } + current = location; } else if (cf.compareTo(Constants.METADATA_LOG_COLUMN_FAMILY) == 0) { String[] split = entry.getValue().toString().split("\\|")[0].split(";"); walogs.add(Arrays.asList(split)); } else if (cf.compareTo(Constants.METADATA_LAST_LOCATION_COLUMN_FAMILY) == 0) { + TServerInstance location = new TServerInstance(entry.getValue(), cq); + if (last != null) { + throw new BadLocationStateException("found two last locations for the same extent " + key.getRow() + ": " + last + " and " + location); + } last = new TServerInstance(entry.getValue(), cq); } else if (cf.compareTo(Constants.METADATA_CHOPPED_COLUMN_FAMILY) == 0) { chopped = true; @@ -161,7 +171,9 @@ public class MetaDataTableScanner implem return createTabletLocationState(e.getKey(), e.getValue()); } catch (IOException ex) { throw new RuntimeException(ex); - } + } catch (BadLocationStateException ex) { + throw new RuntimeException(ex); + } } @Override Modified: accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java URL: http://svn.apache.org/viewvc/accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java?rev=1465630&r1=1465629&r2=1465630&view=diff ============================================================================== --- accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java (original) +++ accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java Mon Apr 8 13:32:16 2013 @@ -21,7 +21,6 @@ import java.util.Collections; import java.util.Set; import org.apache.accumulo.core.data.KeyExtent; -import org.apache.log4j.Logger; /** * When a tablet is assigned, we mark its future location. When the tablet is opened, we set its current location. A tablet should never have both a future and @@ -32,10 +31,14 @@ import org.apache.log4j.Logger; */ public class TabletLocationState { - private static final Logger log = Logger.getLogger(TabletLocationState.class); + static public class BadLocationStateException extends Exception { + private static final long serialVersionUID = 1L; + + BadLocationStateException(String msg) { super(msg); } + } public TabletLocationState(KeyExtent extent, TServerInstance future, TServerInstance current, TServerInstance last, Collection> walogs, - boolean chopped) { + boolean chopped) throws BadLocationStateException { this.extent = extent; this.future = future; this.current = current; @@ -45,7 +48,7 @@ public class TabletLocationState { this.walogs = walogs; this.chopped = chopped; if (current != null && future != null) { - log.error(extent + " is both assigned and hosted, which should never happen: " + this); + throw new BadLocationStateException(extent + " is both assigned and hosted, which should never happen: " + this); } } Modified: accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletStateChangeIterator.java URL: http://svn.apache.org/viewvc/accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletStateChangeIterator.java?rev=1465630&r1=1465629&r2=1465630&view=diff ============================================================================== --- accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletStateChangeIterator.java (original) +++ accumulo/trunk/server/src/main/java/org/apache/accumulo/server/master/state/TabletStateChangeIterator.java Mon Apr 8 13:32:16 2013 @@ -35,6 +35,7 @@ import org.apache.accumulo.core.iterator import org.apache.accumulo.core.iterators.SkippingIterator; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; import org.apache.accumulo.core.util.StringUtil; +import org.apache.accumulo.server.master.state.TabletLocationState.BadLocationStateException; import org.apache.accumulo.server.util.AddressUtil; import org.apache.commons.codec.binary.Base64; import org.apache.hadoop.io.DataInputBuffer; @@ -115,7 +116,13 @@ public class TabletStateChangeIterator e if (onlineTables == null || current == null) return; - TabletLocationState tls = MetaDataTableScanner.createTabletLocationState(k, v); + TabletLocationState tls; + try { + tls = MetaDataTableScanner.createTabletLocationState(k, v); + } catch (BadLocationStateException e) { + // maybe the master can do something with a tablet with bad/inconsistent state + return; + } // we always want data about merges MergeInfo merge = merges.get(tls.extent.getTableId()); if (merge != null && merge.getRange() != null && merge.getRange().overlaps(tls.extent)) { Modified: accumulo/trunk/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java URL: http://svn.apache.org/viewvc/accumulo/trunk/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java?rev=1465630&r1=1465629&r2=1465630&view=diff ============================================================================== --- accumulo/trunk/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java (original) +++ accumulo/trunk/server/src/main/java/org/apache/accumulo/server/tabletserver/TabletServer.java Mon Apr 8 13:32:16 2013 @@ -147,6 +147,7 @@ import org.apache.accumulo.server.master import org.apache.accumulo.server.master.state.DistributedStoreException; import org.apache.accumulo.server.master.state.TServerInstance; import org.apache.accumulo.server.master.state.TabletLocationState; +import org.apache.accumulo.server.master.state.TabletLocationState.BadLocationStateException; import org.apache.accumulo.server.master.state.TabletStateStore; import org.apache.accumulo.server.master.state.ZooTabletStateStore; import org.apache.accumulo.server.metrics.AbstractMetricsImpl; @@ -2369,7 +2370,12 @@ public class TabletServer extends Abstra try { TServerInstance instance = new TServerInstance(clientAddress, getLock().getSessionId()); - TabletLocationState tls = new TabletLocationState(extent, null, instance, null, null, false); + TabletLocationState tls = null; + try { + tls = new TabletLocationState(extent, null, instance, null, null, false); + } catch (BadLocationStateException e) { + log.error("Unexpected error ", e); + } log.debug("Unassigning " + tls); TabletStateStore.unassign(tls); } catch (DistributedStoreException ex) { Modified: accumulo/trunk/server/src/test/java/org/apache/accumulo/server/master/state/RootTabletStateStoreTest.java URL: http://svn.apache.org/viewvc/accumulo/trunk/server/src/test/java/org/apache/accumulo/server/master/state/RootTabletStateStoreTest.java?rev=1465630&r1=1465629&r2=1465630&view=diff ============================================================================== --- accumulo/trunk/server/src/test/java/org/apache/accumulo/server/master/state/RootTabletStateStoreTest.java (original) +++ accumulo/trunk/server/src/test/java/org/apache/accumulo/server/master/state/RootTabletStateStoreTest.java Mon Apr 8 13:32:16 2013 @@ -16,10 +16,10 @@ */ package org.apache.accumulo.server.master.state; -import org.junit.Assert; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Arrays; @@ -30,13 +30,9 @@ import java.util.List; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.data.KeyExtent; import org.apache.accumulo.core.util.AddressUtil; -import org.apache.accumulo.server.master.state.Assignment; -import org.apache.accumulo.server.master.state.DistributedStore; -import org.apache.accumulo.server.master.state.DistributedStoreException; -import org.apache.accumulo.server.master.state.TServerInstance; -import org.apache.accumulo.server.master.state.TabletLocationState; -import org.apache.accumulo.server.master.state.ZooTabletStateStore; +import org.apache.accumulo.server.master.state.TabletLocationState.BadLocationStateException; import org.apache.hadoop.io.Text; +import org.junit.Assert; import org.junit.Test; public class RootTabletStateStoreTest { @@ -172,7 +168,12 @@ public class RootTabletStateStoreTest { count++; } assertEquals(count, 1); - TabletLocationState assigned = new TabletLocationState(root, server, null, null, null, false); + TabletLocationState assigned = null; + try { + assigned = new TabletLocationState(root, server, null, null, null, false); + } catch (BadLocationStateException e) { + fail("Unexpected error " + e); + } tstore.unassign(Collections.singletonList(assigned)); count = 0; for (TabletLocationState location : tstore) { @@ -194,7 +195,12 @@ public class RootTabletStateStoreTest { Assert.fail("should not get here"); } catch (IllegalArgumentException ex) {} - TabletLocationState broken = new TabletLocationState(notRoot, server, null, null, null, false); + TabletLocationState broken = null; + try { + broken = new TabletLocationState(notRoot, server, null, null, null, false); + } catch (BadLocationStateException e) { + fail("Unexpected error " + e); + } try { tstore.unassign(Collections.singletonList(broken)); Assert.fail("should not get here"); Propchange: accumulo/trunk/src/ ------------------------------------------------------------------------------ Merged /accumulo/branches/1.5/src:r1465627