You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by an...@apache.org on 2015/10/10 08:35:13 UTC
hbase git commit: HBASE-14366 NPE in case visibility expression is
not present in labels table during importtsv run. (Bhupendra)
Repository: hbase
Updated Branches:
refs/heads/master 2e593a9d3 -> 4969879df
HBASE-14366 NPE in case visibility expression is not present in labels table during importtsv run. (Bhupendra)
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/4969879d
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/4969879d
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/4969879d
Branch: refs/heads/master
Commit: 4969879df5f6d1d9a2774236cdee1710694ddf6a
Parents: 2e593a9
Author: anoopsjohn <an...@gmail.com>
Authored: Sat Oct 10 12:04:47 2015 +0530
Committer: anoopsjohn <an...@gmail.com>
Committed: Sat Oct 10 12:04:47 2015 +0530
----------------------------------------------------------------------
.../visibility/VisibilityConstants.java | 3 +
.../DefaultVisibilityExpressionResolver.java | 8 +-
.../hadoop/hbase/mapreduce/TextSortReducer.java | 17 ++--
.../hbase/mapreduce/TsvImporterMapper.java | 8 +-
.../visibility/VisibilityLabelsCache.java | 3 +-
.../TestImportTSVWithVisibilityLabels.java | 95 ++++++++++++++++++--
6 files changed, 108 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/4969879d/hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
index 570c203..782d163 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityConstants.java
@@ -58,4 +58,7 @@ public final class VisibilityConstants {
public static final String OPEN_PARAN = "(";
public static final String CLOSED_PARAN = ")";
+ /** Label ordinal value for invalid labels */
+ public static final int NON_EXIST_LABEL_ORDINAL = 0;
+
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/4969879d/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/DefaultVisibilityExpressionResolver.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/DefaultVisibilityExpressionResolver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/DefaultVisibilityExpressionResolver.java
index 70af63a..597a764 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/DefaultVisibilityExpressionResolver.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/DefaultVisibilityExpressionResolver.java
@@ -39,6 +39,7 @@ import org.apache.hadoop.hbase.client.ResultScanner;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.security.visibility.Authorizations;
+import org.apache.hadoop.hbase.security.visibility.VisibilityConstants;
import org.apache.hadoop.hbase.security.visibility.VisibilityLabelOrdinalProvider;
import org.apache.hadoop.hbase.security.visibility.VisibilityUtils;
import org.apache.hadoop.hbase.util.Bytes;
@@ -124,7 +125,12 @@ public class DefaultVisibilityExpressionResolver implements VisibilityExpression
VisibilityLabelOrdinalProvider provider = new VisibilityLabelOrdinalProvider() {
@Override
public int getLabelOrdinal(String label) {
- return labels.get(label);
+ Integer ordinal = null;
+ ordinal = labels.get(label);
+ if (ordinal != null) {
+ return ordinal.intValue();
+ }
+ return VisibilityConstants.NON_EXIST_LABEL_ORDINAL;
}
@Override
http://git-wip-us.apache.org/repos/asf/hbase/blob/4969879d/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TextSortReducer.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TextSortReducer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TextSortReducer.java
index 46e69cd..8fe5cc7 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TextSortReducer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TextSortReducer.java
@@ -24,8 +24,6 @@ import java.util.List;
import java.util.Set;
import java.util.TreeSet;
-import org.apache.hadoop.hbase.classification.InterfaceAudience;
-import org.apache.hadoop.hbase.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellComparator;
@@ -33,7 +31,10 @@ import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.KeyValueUtil;
import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.TagType;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.security.visibility.InvalidLabelException;
import org.apache.hadoop.hbase.util.Base64;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.io.Text;
@@ -186,21 +187,15 @@ public class TextSortReducer extends
kvs.add(kv);
curSize += kv.heapSize();
}
- } catch (ImportTsv.TsvParser.BadTsvLineException badLine) {
+ } catch (ImportTsv.TsvParser.BadTsvLineException | IllegalArgumentException
+ | InvalidLabelException badLine) {
if (skipBadLines) {
System.err.println("Bad line." + badLine.getMessage());
incrementBadLineCount(1);
continue;
}
throw new IOException(badLine);
- } catch (IllegalArgumentException e) {
- if (skipBadLines) {
- System.err.println("Bad line." + e.getMessage());
- incrementBadLineCount(1);
- continue;
- }
- throw new IOException(e);
- }
+ }
}
context.setStatus("Read " + kvs.size() + " entries of " + kvs.getClass()
+ "(" + StringUtils.humanReadableInt(curSize) + ")");
http://git-wip-us.apache.org/repos/asf/hbase/blob/4969879d/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TsvImporterMapper.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TsvImporterMapper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TsvImporterMapper.java
index 2c139c9..98dc25e 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TsvImporterMapper.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/TsvImporterMapper.java
@@ -21,17 +21,18 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
-import org.apache.hadoop.hbase.classification.InterfaceAudience;
-import org.apache.hadoop.hbase.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.TagType;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
import org.apache.hadoop.hbase.client.Put;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.hadoop.hbase.mapreduce.ImportTsv.TsvParser.BadTsvLineException;
import org.apache.hadoop.hbase.security.visibility.CellVisibility;
+import org.apache.hadoop.hbase.security.visibility.InvalidLabelException;
import org.apache.hadoop.hbase.util.Base64;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.io.LongWritable;
@@ -182,7 +183,8 @@ extends Mapper<LongWritable, Text, ImmutableBytesWritable, Put>
populatePut(lineBytes, parsed, put, i);
}
context.write(rowKey, put);
- } catch (ImportTsv.TsvParser.BadTsvLineException|IllegalArgumentException badLine) {
+ } catch (ImportTsv.TsvParser.BadTsvLineException | IllegalArgumentException
+ | InvalidLabelException badLine) {
if (logBadLines) {
System.err.println(value);
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/4969879d/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelsCache.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelsCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelsCache.java
index 763a624..0948520 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelsCache.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelsCache.java
@@ -49,7 +49,6 @@ import org.apache.zookeeper.KeeperException;
public class VisibilityLabelsCache implements VisibilityLabelOrdinalProvider {
private static final Log LOG = LogFactory.getLog(VisibilityLabelsCache.class);
- private static final int NON_EXIST_LABEL_ORDINAL = 0;
private static final List<String> EMPTY_LIST = Collections.emptyList();
private static final Set<Integer> EMPTY_SET = Collections.emptySet();
private static VisibilityLabelsCache instance;
@@ -175,7 +174,7 @@ public class VisibilityLabelsCache implements VisibilityLabelOrdinalProvider {
return ordinal.intValue();
}
// 0 denotes not available
- return NON_EXIST_LABEL_ORDINAL;
+ return VisibilityConstants.NON_EXIST_LABEL_ORDINAL;
}
/**
http://git-wip-us.apache.org/repos/asf/hbase/blob/4969879d/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
index 6f0696e..3aec669 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTSVWithVisibilityLabels.java
@@ -42,18 +42,16 @@ import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
-import org.apache.hadoop.hbase.client.Admin;
-import org.apache.hadoop.hbase.testclassification.LargeTests;
-import org.apache.hadoop.hbase.testclassification.MapReduceTests;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.client.ConnectionFactory;
import org.apache.hadoop.hbase.client.Delete;
-import org.apache.hadoop.hbase.client.HBaseAdmin;
-import org.apache.hadoop.hbase.client.HTable;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.ResultScanner;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFile;
+import org.apache.hadoop.hbase.io.hfile.HFileScanner;
import org.apache.hadoop.hbase.protobuf.generated.VisibilityLabelsProtos.VisibilityLabelsResponse;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.security.visibility.Authorizations;
@@ -64,6 +62,8 @@ import org.apache.hadoop.hbase.security.visibility.VisibilityClient;
import org.apache.hadoop.hbase.security.visibility.VisibilityConstants;
import org.apache.hadoop.hbase.security.visibility.VisibilityController;
import org.apache.hadoop.hbase.security.visibility.VisibilityUtils;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.testclassification.MapReduceTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.mapred.Utils.OutputFileUtils.OutputFilesFilter;
import org.apache.hadoop.util.Tool;
@@ -274,6 +274,50 @@ public class TestImportTSVWithVisibilityLabels implements Configurable {
util.deleteTable(tableName);
}
+ @Test
+ public void testBulkOutputWithInvalidLabels() throws Exception {
+ TableName tableName = TableName.valueOf("test-" + UUID.randomUUID());
+ Path hfiles = new Path(util.getDataTestDirOnTestFS(tableName.getNameAsString()), "hfiles");
+ // Prepare the arguments required for the test.
+ String[] args =
+ new String[] { "-D" + ImportTsv.BULK_OUTPUT_CONF_KEY + "=" + hfiles.toString(),
+ "-D" + ImportTsv.COLUMNS_CONF_KEY + "=HBASE_ROW_KEY,FAM:A,FAM:B,HBASE_CELL_VISIBILITY",
+ "-D" + ImportTsv.SEPARATOR_CONF_KEY + "=\u001b", tableName.getNameAsString() };
+
+ // 2 Data rows, one with valid label and one with invalid label
+ String data =
+ "KEY\u001bVALUE1\u001bVALUE2\u001bprivate\nKEY1\u001bVALUE1\u001bVALUE2\u001binvalid\n";
+ util.createTable(tableName, FAMILY);
+ doMROnTableTest(util, FAMILY, data, args, 1, 2);
+ util.deleteTable(tableName);
+ }
+
+ @Test
+ public void testBulkOutputWithTsvImporterTextMapperWithInvalidLabels() throws Exception {
+ TableName tableName = TableName.valueOf("test-" + UUID.randomUUID());
+ Path hfiles = new Path(util.getDataTestDirOnTestFS(tableName.getNameAsString()), "hfiles");
+ // Prepare the arguments required for the test.
+ String[] args =
+ new String[] {
+ "-D" + ImportTsv.MAPPER_CONF_KEY
+ + "=org.apache.hadoop.hbase.mapreduce.TsvImporterTextMapper",
+ "-D" + ImportTsv.BULK_OUTPUT_CONF_KEY + "=" + hfiles.toString(),
+ "-D" + ImportTsv.COLUMNS_CONF_KEY + "=HBASE_ROW_KEY,FAM:A,FAM:B,HBASE_CELL_VISIBILITY",
+ "-D" + ImportTsv.SEPARATOR_CONF_KEY + "=\u001b", tableName.getNameAsString() };
+
+ // 2 Data rows, one with valid label and one with invalid label
+ String data =
+ "KEY\u001bVALUE1\u001bVALUE2\u001bprivate\nKEY1\u001bVALUE1\u001bVALUE2\u001binvalid\n";
+ util.createTable(tableName, FAMILY);
+ doMROnTableTest(util, FAMILY, data, args, 1, 2);
+ util.deleteTable(tableName);
+ }
+
+ protected static Tool doMROnTableTest(HBaseTestingUtility util, String family, String data,
+ String[] args, int valueMultiplier) throws Exception {
+ return doMROnTableTest(util, family, data, args, valueMultiplier, -1);
+ }
+
/**
* Run an ImportTsv job and perform basic validation on the results. Returns
* the ImportTsv <code>Tool</code> instance so that other tests can inspect it
@@ -282,10 +326,13 @@ public class TestImportTSVWithVisibilityLabels implements Configurable {
*
* @param args
* Any arguments to pass BEFORE inputFile path is appended.
+ *
+ * @param expectedKVCount Expected KV count. pass -1 to skip the kvcount check
+ *
* @return The Tool instance used to run the test.
*/
protected static Tool doMROnTableTest(HBaseTestingUtility util, String family, String data,
- String[] args, int valueMultiplier) throws Exception {
+ String[] args, int valueMultiplier,int expectedKVCount) throws Exception {
TableName table = TableName.valueOf(args[args.length - 1]);
Configuration conf = new Configuration(util.getConfiguration());
@@ -328,7 +375,7 @@ public class TestImportTSVWithVisibilityLabels implements Configurable {
}
LOG.debug("validating the table " + createdHFiles);
if (createdHFiles)
- validateHFiles(fs, outputPath, family);
+ validateHFiles(fs, outputPath, family,expectedKVCount);
else
validateTable(conf, table, family, valueMultiplier);
@@ -342,14 +389,15 @@ public class TestImportTSVWithVisibilityLabels implements Configurable {
/**
* Confirm ImportTsv via HFiles on fs.
*/
- private static void validateHFiles(FileSystem fs, String outputPath, String family)
- throws IOException {
+ private static void validateHFiles(FileSystem fs, String outputPath, String family,
+ int expectedKVCount) throws IOException {
// validate number and content of output columns
LOG.debug("Validating HFiles.");
Set<String> configFamilies = new HashSet<String>();
configFamilies.add(family);
Set<String> foundFamilies = new HashSet<String>();
+ int actualKVCount = 0;
for (FileStatus cfStatus : fs.listStatus(new Path(outputPath), new OutputFilesFilter())) {
LOG.debug("The output path has files");
String[] elements = cfStatus.getPath().toString().split(Path.SEPARATOR);
@@ -361,8 +409,16 @@ public class TestImportTSVWithVisibilityLabels implements Configurable {
for (FileStatus hfile : fs.listStatus(cfStatus.getPath())) {
assertTrue(String.format("HFile %s appears to contain no data.", hfile.getPath()),
hfile.getLen() > 0);
+ if (expectedKVCount > -1) {
+ actualKVCount += getKVCountFromHfile(fs, hfile.getPath());
+ }
}
}
+ if (expectedKVCount > -1) {
+ assertTrue(String.format(
+ "KV count in output hfile=<%d> doesn't match with expected KV count=<%d>", actualKVCount,
+ expectedKVCount), actualKVCount == expectedKVCount);
+ }
}
/**
@@ -412,4 +468,25 @@ public class TestImportTSVWithVisibilityLabels implements Configurable {
assertTrue(verified);
}
+ /**
+ * Method returns the total KVs in given hfile
+ * @param fs File System
+ * @param p HFile path
+ * @return KV count in the given hfile
+ * @throws IOException
+ */
+ private static int getKVCountFromHfile(FileSystem fs, Path p) throws IOException {
+ Configuration conf = util.getConfiguration();
+ HFile.Reader reader = HFile.createReader(fs, p, new CacheConfig(conf), conf);
+ reader.loadFileInfo();
+ HFileScanner scanner = reader.getScanner(false, false);
+ scanner.seekTo();
+ int count = 0;
+ do {
+ count++;
+ } while (scanner.next());
+ reader.close();
+ return count;
+ }
+
}