This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch 2.0
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.0 by this push:
new 422a9d8 correctly handle first and last tablets in LinkingIterator
#1260 (#1266)
422a9d8 is described below
commit 422a9d88cea0061533ef9c087d69e0123721c049
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Thu Jul 11 17:58:47 2019 -0400
correctly handle first and last tablets in LinkingIterator #1260 (#1266)
The LinkingIterator checks the structure of the AccumuloMetadata table
as it is scans. It was not properly handling a split of the first
tablet. It was also not ensuring the default tablet for a table was seen.
This patch fixes those two issues. These issues were discovered while
researching #1260, however I am not sure if these issues could have
caused #1260.
---
.../core/metadata/schema/LinkingIterator.java | 40 +++++++++++++--
.../core/metadata/schema/LinkingIteratorTest.java | 59 ++++++++++++++++++++--
2 files changed, 93 insertions(+), 6 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/LinkingIterator.java
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/LinkingIterator.java
index d12e34c..15ac4ce 100644
---
a/core/src/main/java/org/apache/accumulo/core/metadata/schema/LinkingIterator.java
+++
b/core/src/main/java/org/apache/accumulo/core/metadata/schema/LinkingIterator.java
@@ -26,6 +26,7 @@ import java.util.function.Function;
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.PartialKey;
import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection;
import org.apache.hadoop.io.Text;
import org.slf4j.Logger;
@@ -62,7 +63,20 @@ public class LinkingIterator implements
Iterator<TabletMetadata> {
@Override
public boolean hasNext() {
- return source.hasNext();
+ boolean hasNext = source.hasNext();
+
+ // Always expect the default tablet to exist for a table. The following
checks for the case when
+ // the default tablet was not seen when it should have been seen.
+ if (!hasNext && prevTablet != null && prevTablet.getEndRow() != null) {
+ Text defaultTabletRow = TabletsSection.getRow(prevTablet.getTableId(),
null);
+ if (range.contains(new Key(defaultTabletRow))) {
+ throw new IllegalStateException(
+ "Scan range incudled default tablet, but did not see default
tablet. Last tablet seen : "
+ + prevTablet.getExtent());
+ }
+ }
+
+ return hasNext;
}
static boolean goodTransition(TabletMetadata prev, TabletMetadata curr) {
@@ -101,6 +115,7 @@ public class LinkingIterator implements
Iterator<TabletMetadata> {
private void resetSource() {
if (prevTablet == null) {
+ log.debug("Resetting scanner to {}", range);
source = iteratorFactory.apply(range);
} else {
// get the metadata table row for the previous tablet
@@ -115,7 +130,7 @@ public class LinkingIterator implements
Iterator<TabletMetadata> {
Range seekRange = new Range(new
Key(prevMetaRow).followingKey(PartialKey.ROW), true,
range.getEndKey(), range.isEndKeyInclusive());
- log.info("Resetting scanner to {}", seekRange);
+ log.debug("Resetting scanner to {}", seekRange);
source = iteratorFactory.apply(seekRange);
}
@@ -132,7 +147,26 @@ public class LinkingIterator implements
Iterator<TabletMetadata> {
if (prevTablet == null) {
if (tmp.sawPrevEndRow()) {
- currTablet = tmp;
+
+ Text prevMetaRow = null;
+
+ KeyExtent extent = tmp.getExtent();
+
+ if (extent.getPrevEndRow() != null) {
+ prevMetaRow = TabletsSection.getRow(extent.getTableId(),
extent.getPrevEndRow());
+ }
+
+ // If the first tablet seen has a prev endrow within the range it
means a preceding tablet
+ // exists that we need to go back and read. This could be caused by
the first tablet in
+ // the range splitting concurrently while this code is reading.
+
+ if (prevMetaRow == null || range.beforeStartKey(new
Key(prevMetaRow))) {
+ currTablet = tmp;
+ } else {
+ log.debug(
+ "First tablet seen provides evidence of earlier tablet in
range, retrying {} {} ",
+ prevMetaRow, range);
+ }
} else {
log.warn("Tablet has no prev end row " + tmp.getTableId() + " " +
tmp.getEndRow());
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/LinkingIteratorTest.java
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/LinkingIteratorTest.java
index af9a687..371f6bb 100644
---
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/LinkingIteratorTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/LinkingIteratorTest.java
@@ -29,7 +29,10 @@ import java.util.stream.Stream;
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.Range;
+import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection;
+import org.apache.hadoop.io.Text;
import org.junit.Test;
import com.google.common.collect.Lists;
@@ -55,13 +58,21 @@ public class LinkingIteratorTest {
}
}
- private static void check(List<TabletMetadata> expected, IterFactory
iterFactory) {
+ private static void check(List<TabletMetadata> expected, IterFactory
iterFactory, Range range) {
List<KeyExtent> actual = new ArrayList<>();
- new LinkingIterator(iterFactory, new Range())
- .forEachRemaining(tm -> actual.add(tm.getExtent()));
+ new LinkingIterator(iterFactory, range).forEachRemaining(tm ->
actual.add(tm.getExtent()));
assertEquals(Lists.transform(expected, TabletMetadata::getExtent), actual);
}
+ private static void check(List<TabletMetadata> expected, IterFactory
iterFactory) {
+ check(expected, iterFactory, new Range());
+ }
+
+ private static void check(List<TabletMetadata> expected, IterFactory
iterFactory,
+ TableId tableId) {
+ check(expected, iterFactory, TabletsSection.getRange(tableId));
+ }
+
@Test
public void testHole() {
@@ -109,4 +120,46 @@ public class LinkingIteratorTest {
check(tablets2, new IterFactory(tablets1, tablets2));
}
+
+ @Test
+ public void testFirstTabletSplits() {
+ // check when first tablet has a prev end row that points to a non
existent tablet. This could
+ // be caused by the first table splitting concurrently with a metadata
scan of the first tablet.
+ List<TabletMetadata> tablets1 = Arrays.asList(create("4", "f", "m"),
create("4", "m", null));
+ List<TabletMetadata> tablets2 =
+ Arrays.asList(create("4", null, "f"), create("4", "f", "m"),
create("4", "m", null));
+
+ check(tablets2, new IterFactory(tablets1, tablets2), TableId.of("4"));
+ check(tablets2, new IterFactory(tablets1, tablets2),
+ new KeyExtent(TableId.of("4"), null, new Text("e")).toMetadataRange());
+
+ // following should not care about missing tablet
+ check(tablets1, new IterFactory(tablets1, tablets2),
+ new KeyExtent(TableId.of("4"), null, new Text("g")).toMetadataRange());
+ check(tablets1, new IterFactory(tablets1, tablets2),
+ new KeyExtent(TableId.of("4"), null, new Text("f")).toMetadataRange());
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testIncompleteTable() {
+ // the last tablet in a table should have a null end row. Ensure the code
detects when this does
+ // not happen.
+ List<TabletMetadata> tablets1 = Arrays.asList(create("4", null, "f"),
create("4", "f", "m"));
+
+ LinkingIterator li = new LinkingIterator(new IterFactory(tablets1,
tablets1),
+ TabletsSection.getRange(TableId.of("4")));
+
+ while (li.hasNext()) {
+ li.next();
+ }
+ }
+
+ @Test
+ public void testIncompleteTableWithRange() {
+ // because the scan range does not got to end of table, this should not
care about missing
+ // tablets at end of table.
+ List<TabletMetadata> tablets1 = Arrays.asList(create("4", null, "f"),
create("4", "f", "m"));
+ check(tablets1, new IterFactory(tablets1, tablets1),
+ new KeyExtent(TableId.of("4"), new Text("r"), new
Text("e")).toMetadataRange());
+ }
}
