From 34532dcc620fc638685cebd9cbe222ec3f9a5b93 Mon Sep 17 00:00:00 2001 From: Mike Miller Date: Wed, 29 Mar 2017 16:45:56 -0400 Subject: [PATCH] ACCUMULO-3521: created tests and minor improvements for iterators: modified: core/src/main/java/org/apache/accumulo/core/iterators/FirstEntryInRowIterator.java modified: core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java modified: core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java modified: core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java --- .../iterators/FirstEntryInRowIterator.java | 13 ++---- .../core/iterators/TypedValueCombiner.java | 11 +---- .../FirstEntryInRowIteratorTest.java | 22 +++++---- .../core/iterators/user/CombinerTest.java | 46 +++++++++++++++++++ 4 files changed, 67 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/FirstEntryInRowIterator.java b/core/src/main/java/org/apache/accumulo/core/iterators/FirstEntryInRowIterator.java index 32e6464831d..17b8f438438 100644 --- a/core/src/main/java/org/apache/accumulo/core/iterators/FirstEntryInRowIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/iterators/FirstEntryInRowIterator.java @@ -28,6 +28,7 @@ import org.apache.accumulo.core.data.Range; import org.apache.accumulo.core.data.Value; import org.apache.hadoop.io.Text; +import org.apache.commons.lang.math.NumberUtils; public class FirstEntryInRowIterator extends SkippingIterator implements OptionDescriber { @@ -75,7 +76,7 @@ public void init(SortedKeyValueIterator source, Map op // this is only ever called immediately after getting "next" entry @Override protected void consume() throws IOException { - if (finished == true || lastRowFound == null) + if (finished || lastRowFound == null) return; int count = 0; while (getSource().hasTop() && lastRowFound.equals(getSource().getTopKey().getRow())) { @@ -139,13 +140,9 @@ public IteratorOptions describeOptions() { @Override public boolean validateOptions(Map options) { - try { - String o = options.get(NUM_SCANS_STRING_NAME); - if (o != null) - Integer.parseInt(o); - } catch (Exception e) { - throw new IllegalArgumentException("bad integer " + NUM_SCANS_STRING_NAME + ":" + options.get(NUM_SCANS_STRING_NAME), e); - } + String o = options.get(NUM_SCANS_STRING_NAME); + if (o != null && !NumberUtils.isNumber(o)) + throw new IllegalArgumentException("bad integer " + NUM_SCANS_STRING_NAME + ":" + options.get(NUM_SCANS_STRING_NAME)); return true; } diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java b/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java index 03e4d88f0e6..05e39550096 100644 --- a/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java +++ b/core/src/main/java/org/apache/accumulo/core/iterators/TypedValueCombiner.java @@ -131,11 +131,7 @@ protected void setEncoder(String encoderClass) { @SuppressWarnings("unchecked") Class> clazz = (Class>) AccumuloVFSClassLoader.loadClass(encoderClass, Encoder.class); encoder = clazz.newInstance(); - } catch (ClassNotFoundException e) { - throw new IllegalArgumentException(e); - } catch (InstantiationException e) { - throw new IllegalArgumentException(e); - } catch (IllegalAccessException e) { + } catch (ClassNotFoundException | IllegalAccessException | InstantiationException e) { throw new IllegalArgumentException(e); } } @@ -190,10 +186,7 @@ public void init(SortedKeyValueIterator source, Map op private void setLossyness(Map options) { String loss = options.get(LOSSY); - if (loss == null) - lossy = false; - else - lossy = Boolean.parseBoolean(loss); + lossy = loss != null && Boolean.parseBoolean(loss); } @Override diff --git a/core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java b/core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java index 34b01bca01c..29561520af9 100644 --- a/core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java +++ b/core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java @@ -17,31 +17,33 @@ package org.apache.accumulo.core.iterators; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.util.Collections; import java.util.TreeMap; +import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.client.impl.BaseIteratorEnvironment; 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.data.Value; import org.apache.accumulo.core.iterators.system.CountingIterator; +import org.apache.accumulo.core.util.LocalityGroupUtil; import org.junit.Test; public class FirstEntryInRowIteratorTest { @SuppressWarnings("unchecked") - private static long process(TreeMap sourceMap, TreeMap resultMap, Range range, int numScans) throws IOException { + private static long process(TreeMap sourceMap, TreeMap resultMap, Range range, IteratorSetting iteratorSetting) throws IOException { org.apache.accumulo.core.iterators.SortedMapIterator source = new SortedMapIterator(sourceMap); CountingIterator counter = new CountingIterator(source); FirstEntryInRowIterator feiri = new FirstEntryInRowIterator(); IteratorEnvironment env = new BaseIteratorEnvironment(); - feiri.init(counter, Collections.singletonMap(FirstEntryInRowIterator.NUM_SCANS_STRING_NAME, Integer.toString(numScans)), env); + feiri.init(counter, iteratorSetting.getOptions(), env); - feiri.seek(range, Collections.EMPTY_SET, false); + feiri.seek(range, LocalityGroupUtil.EMPTY_CF_SET, false); while (feiri.hasTop()) { resultMap.put(feiri.getTopKey(), feiri.getTopValue()); feiri.next(); @@ -53,12 +55,15 @@ private static long process(TreeMap sourceMap, TreeMap res public void test() throws IOException { TreeMap sourceMap = new TreeMap<>(); Value emptyValue = new Value("".getBytes()); + IteratorSetting iteratorSetting = new IteratorSetting(1, FirstEntryInRowIterator.class); + FirstEntryInRowIterator.setNumScansBeforeSeek(iteratorSetting, 10); + assertTrue(iteratorSetting.getOptions().containsKey(FirstEntryInRowIterator.NUM_SCANS_STRING_NAME)); sourceMap.put(new Key("r1", "cf", "cq"), emptyValue); sourceMap.put(new Key("r2", "cf", "cq"), emptyValue); sourceMap.put(new Key("r3", "cf", "cq"), emptyValue); TreeMap resultMap = new TreeMap<>(); long numSourceEntries = sourceMap.size(); - long numNexts = process(sourceMap, resultMap, new Range(), 10); + long numNexts = process(sourceMap, resultMap, new Range(), iteratorSetting); assertEquals(numNexts, numSourceEntries); assertEquals(sourceMap.size(), resultMap.size()); @@ -66,17 +71,18 @@ public void test() throws IOException { sourceMap.put(new Key("r2", "cf", "cq" + i), emptyValue); } resultMap.clear(); - numNexts = process(sourceMap, resultMap, new Range(new Key("r1"), (new Key("r2")).followingKey(PartialKey.ROW)), 10); + + numNexts = process(sourceMap, resultMap, new Range(new Key("r1"), (new Key("r2")).followingKey(PartialKey.ROW)), iteratorSetting); assertEquals(numNexts, resultMap.size() + 10); assertEquals(resultMap.size(), 2); resultMap.clear(); - numNexts = process(sourceMap, resultMap, new Range(new Key("r1"), new Key("r2", "cf2")), 10); + numNexts = process(sourceMap, resultMap, new Range(new Key("r1"), new Key("r2", "cf2")), iteratorSetting); assertEquals(numNexts, resultMap.size() + 10); assertEquals(resultMap.size(), 2); resultMap.clear(); - numNexts = process(sourceMap, resultMap, new Range(new Key("r1"), new Key("r4")), 10); + numNexts = process(sourceMap, resultMap, new Range(new Key("r1"), new Key("r4")), iteratorSetting); assertEquals(numNexts, resultMap.size() + 10); assertEquals(resultMap.size(), 3); } diff --git a/core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java b/core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java index ec740a6b4b4..c996f5d93d1 100644 --- a/core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/iterators/user/CombinerTest.java @@ -48,6 +48,7 @@ import org.apache.accumulo.core.iterators.SortedMapIterator; import org.apache.accumulo.core.iterators.TypedValueCombiner; import org.apache.accumulo.core.iterators.TypedValueCombiner.Encoder; +import org.apache.accumulo.core.iterators.ValueFormatException; import org.apache.accumulo.core.iterators.system.MultiIterator; import org.apache.hadoop.io.Text; import org.apache.log4j.Logger; @@ -922,4 +923,49 @@ public void testDeleteHandling() throws Exception { runDeleteHandlingTest(input, expected, null, paritalMajcIe, ".*ERROR.*SummingCombiner.*ACCUMULO-2232.*"); runDeleteHandlingTest(input, expected, null, fullMajcIe, ".*ERROR.*SummingCombiner.*ACCUMULO-2232.*"); } + + /** + * Tests the Lossy option will ignore errors in TypedValueCombiner. Uses SummingArrayCombiner to generate error. + */ + @Test + public void testLossyOption() throws IOException, IllegalAccessException, InstantiationException { + Encoder> encoder = new SummingArrayCombiner.VarLongArrayEncoder(); + + TreeMap tm1 = new TreeMap<>(); + + // keys that aggregate + tm1.put(newKey(1, 1, 1, 1, false), new Value("badValue")); + newKeyValue(tm1, 1, 1, 1, 2, false, nal(3l, 4l, 5l), encoder); + newKeyValue(tm1, 1, 1, 1, 3, false, nal(), encoder); + + SummingArrayCombiner summingArrayCombiner = new SummingArrayCombiner(); + IteratorSetting iteratorSetting = new IteratorSetting(1, SummingArrayCombiner.class); + SummingArrayCombiner.setEncodingType(iteratorSetting, SummingArrayCombiner.Type.VARLEN); + Combiner.setColumns(iteratorSetting, Collections.singletonList(new IteratorSetting.Column("cf001"))); + + // lossy = true so ignore bad value + TypedValueCombiner.setLossyness(iteratorSetting, true); + assertTrue(summingArrayCombiner.validateOptions(iteratorSetting.getOptions())); + + summingArrayCombiner.init(new SortedMapIterator(tm1), iteratorSetting.getOptions(), SCAN_IE); + summingArrayCombiner.seek(new Range(), EMPTY_COL_FAMS, false); + + assertTrue(summingArrayCombiner.hasTop()); + assertEquals(newKey(1, 1, 1, 3), summingArrayCombiner.getTopKey()); + assertBytesEqual(encoder.encode(nal(3l, 4l, 5l)), summingArrayCombiner.getTopValue().get()); + + summingArrayCombiner.next(); + + assertFalse(summingArrayCombiner.hasTop()); + + // lossy = false throw error for bad value + TypedValueCombiner.setLossyness(iteratorSetting, false); + assertTrue(summingArrayCombiner.validateOptions(iteratorSetting.getOptions())); + + summingArrayCombiner.init(new SortedMapIterator(tm1), iteratorSetting.getOptions(), SCAN_IE); + try { + summingArrayCombiner.seek(new Range(), EMPTY_COL_FAMS, false); + Assert.fail("ValueFormatException should have been thrown"); + } catch (ValueFormatException e) {} + } }