HBASE-19471 Fixed remaining Checkstyle errors in hbase-thrift
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/83017960 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/83017960 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/83017960 Branch: refs/heads/HBASE-19397 Commit: 830179600df8b4f254709aaf4cbf6afc9a548268 Parents: 228d7a5 Author: Jan Hentschel <[email protected]> Authored: Sat Dec 9 20:17:23 2017 +0100 Committer: Jan Hentschel <[email protected]> Committed: Sun Jan 7 13:14:00 2018 +0100 ---------------------------------------------------------------------- .../resources/hbase/checkstyle-suppressions.xml | 1 + hbase-thrift/pom.xml | 16 + .../hbase/thrift/HbaseHandlerMetricsProxy.java | 2 +- .../hadoop/hbase/thrift/IncrementCoalescer.java | 56 +- .../hbase/thrift/TBoundedThreadPoolServer.java | 2 +- .../hadoop/hbase/thrift/ThriftMetrics.java | 8 +- .../hadoop/hbase/thrift/ThriftServer.java | 37 +- .../hadoop/hbase/thrift/ThriftServerRunner.java | 100 +-- .../hadoop/hbase/thrift/ThriftUtilities.java | 61 +- .../hadoop/hbase/thrift2/ThriftServer.java | 236 +++--- .../hadoop/hbase/thrift2/ThriftUtilities.java | 71 +- .../hadoop/hbase/thrift/TestCallQueue.java | 2 +- .../hbase/thrift/TestThriftHttpServer.java | 2 +- .../hadoop/hbase/thrift/TestThriftServer.java | 73 +- .../hbase/thrift/TestThriftServerCmdLine.java | 5 +- .../thrift2/TestThriftHBaseServiceHandler.java | 25 +- ...TestThriftHBaseServiceHandlerWithLabels.java | 711 ++++++++++--------- 17 files changed, 740 insertions(+), 668 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml ---------------------------------------------------------------------- diff --git a/hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml b/hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml index 07261bf..b4173e0 100644 --- a/hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml +++ b/hbase-checkstyle/src/main/resources/hbase/checkstyle-suppressions.xml @@ -36,4 +36,5 @@ <suppress checks="MagicNumberCheck" files=".*/src/test/.*\.java"/> <suppress checks="VisibilityModifier" files=".*/src/test/.*\.java"/> <suppress checks="InterfaceIsTypeCheck" files=".*/src/main/.*\.java"/> + <suppress checks="EmptyBlockCheck" files="TBoundedThreadPoolServer.java"/> </suppressions> http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/pom.xml ---------------------------------------------------------------------- diff --git a/hbase-thrift/pom.xml b/hbase-thrift/pom.xml index 15a7a0c..a74d6b3 100644 --- a/hbase-thrift/pom.xml +++ b/hbase-thrift/pom.xml @@ -139,6 +139,22 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-source-plugin</artifactId> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-checkstyle-plugin</artifactId> + <executions> + <execution> + <id>checkstyle</id> + <phase>validate</phase> + <goals> + <goal>check</goal> + </goals> + <configuration> + <failOnViolation>true</failOnViolation> + </configuration> + </execution> + </executions> + </plugin> </plugins> <pluginManagement> <plugins> http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java index 46e3943..5a6e436 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java @@ -34,7 +34,7 @@ import org.slf4j.LoggerFactory; * time of each call to ThriftMetrics. */ @InterfaceAudience.Private -public class HbaseHandlerMetricsProxy implements InvocationHandler { +public final class HbaseHandlerMetricsProxy implements InvocationHandler { private static final Logger LOG = LoggerFactory.getLogger( HbaseHandlerMetricsProxy.class); http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/IncrementCoalescer.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/IncrementCoalescer.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/IncrementCoalescer.java index 60a8b7f..0dacf8b 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/IncrementCoalescer.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/IncrementCoalescer.java @@ -117,14 +117,30 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { @Override public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null) return false; - if (getClass() != obj.getClass()) return false; + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + FullyQualifiedRow other = (FullyQualifiedRow) obj; - if (!Arrays.equals(family, other.family)) return false; - if (!Arrays.equals(qualifier, other.qualifier)) return false; - if (!Arrays.equals(rowKey, other.rowKey)) return false; - if (!Arrays.equals(table, other.table)) return false; + + if (!Arrays.equals(family, other.family)) { + return false; + } + if (!Arrays.equals(qualifier, other.qualifier)) { + return false; + } + if (!Arrays.equals(rowKey, other.rowKey)) { + return false; + } + if (!Arrays.equals(table, other.table)) { + return false; + } return true; } @@ -144,8 +160,14 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { public Thread newThread(Runnable r) { Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement(), 0); - if (!t.isDaemon()) t.setDaemon(true); - if (t.getPriority() != Thread.NORM_PRIORITY) t.setPriority(Thread.NORM_PRIORITY); + + if (!t.isDaemon()) { + t.setDaemon(true); + } + if (t.getPriority() != Thread.NORM_PRIORITY) { + t.setPriority(Thread.NORM_PRIORITY); + } + return t; } } @@ -191,13 +213,16 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { for (TIncrement tinc : incs) { internalQueueTincrement(tinc); } - return true; + return true; } private boolean internalQueueTincrement(TIncrement inc) throws TException { byte[][] famAndQf = CellUtil.parseColumn(inc.getColumn()); - if (famAndQf.length != 2) return false; + + if (famAndQf.length != 2) { + return false; + } return internalQueueIncrement(inc.getTable(), inc.getRow(), famAndQf[0], famAndQf[1], inc.getAmmount()); @@ -207,7 +232,6 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { byte[] qual, long ammount) throws TException { int countersMapSize = countersMap.size(); - //Make sure that the number of threads is scaled. dynamicallySetCoreSize(countersMapSize); @@ -293,7 +317,7 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { /** * This method samples the incoming requests and, if selected, will check if * the corePoolSize should be changed. - * @param countersMapSize + * @param countersMapSize the size of the counters map */ private void dynamicallySetCoreSize(int countersMapSize) { // Here we are using countersMapSize as a random number, meaning this @@ -302,9 +326,10 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { return; } double currentRatio = (double) countersMapSize / (double) maxQueueSize; - int newValue = 1; + int newValue; + if (currentRatio < 0.1) { - // it's 1 + newValue = 1; } else if (currentRatio < 0.3) { newValue = 2; } else if (currentRatio < 0.5) { @@ -316,6 +341,7 @@ public class IncrementCoalescer implements IncrementCoalescerMBean { } else { newValue = 22; } + if (pool.getCorePoolSize() != newValue) { pool.setCorePoolSize(newValue); } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/TBoundedThreadPoolServer.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/TBoundedThreadPoolServer.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/TBoundedThreadPoolServer.java index 732e282..4926c8b 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/TBoundedThreadPoolServer.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/TBoundedThreadPoolServer.java @@ -258,7 +258,7 @@ public class TBoundedThreadPoolServer extends TServer { serverTransport_.interrupt(); } - private class ClientConnnection implements Runnable { + private final class ClientConnnection implements Runnable { private TTransport client; http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java index 1009210..f612eeb 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java @@ -64,12 +64,14 @@ public class ThriftMetrics { public ThriftMetrics(Configuration conf, ThriftServerType t) { - slowResponseTime = conf.getLong( SLOW_RESPONSE_NANO_SEC, DEFAULT_SLOW_RESPONSE_NANO_SEC); + slowResponseTime = conf.getLong(SLOW_RESPONSE_NANO_SEC, DEFAULT_SLOW_RESPONSE_NANO_SEC); if (t == ThriftServerType.ONE) { - source = CompatibilitySingletonFactory.getInstance(MetricsThriftServerSourceFactory.class).createThriftOneSource(); + source = CompatibilitySingletonFactory.getInstance(MetricsThriftServerSourceFactory.class) + .createThriftOneSource(); } else if (t == ThriftServerType.TWO) { - source = CompatibilitySingletonFactory.getInstance(MetricsThriftServerSourceFactory.class).createThriftTwoSource(); + source = CompatibilitySingletonFactory.getInstance(MetricsThriftServerSourceFactory.class) + .createThriftTwoSource(); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java index cd1993d..b6051d8 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServer.java @@ -87,23 +87,24 @@ public class ThriftServer { /** * Start up or shuts down the Thrift server, depending on the arguments. - * @param args + * @param args the arguments to pass in when starting the Thrift server */ - void doMain(final String[] args) throws Exception { - processOptions(args); - - serverRunner = new ThriftServerRunner(conf); - - // Put up info server. - int port = conf.getInt("hbase.thrift.info.port", 9095); - if (port >= 0) { - conf.setLong("startcode", System.currentTimeMillis()); - String a = conf.get("hbase.thrift.info.bindAddress", "0.0.0.0"); - infoServer = new InfoServer("thrift", a, port, false, conf); - infoServer.setAttribute("hbase.conf", conf); - infoServer.start(); - } - serverRunner.run(); + void doMain(final String[] args) throws Exception { + processOptions(args); + serverRunner = new ThriftServerRunner(conf); + + // Put up info server. + int port = conf.getInt("hbase.thrift.info.port", 9095); + + if (port >= 0) { + conf.setLong("startcode", System.currentTimeMillis()); + String a = conf.get("hbase.thrift.info.bindAddress", "0.0.0.0"); + infoServer = new InfoServer("thrift", a, port, false, conf); + infoServer.setAttribute("hbase.conf", conf); + infoServer.start(); + } + + serverRunner.run(); } /** @@ -230,10 +231,6 @@ public class ThriftServer { } } - /** - * @param args - * @throws Exception - */ public static void main(String [] args) throws Exception { LOG.info("***** STARTING service '" + ThriftServer.class.getSimpleName() + "' *****"); VersionInfo.logVersion(); http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java index 0060181..583a9e9 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java @@ -161,7 +161,8 @@ public class ThriftServerRunner implements Runnable { static final String BIND_CONF_KEY = "hbase.regionserver.thrift.ipaddress"; static final String COMPACT_CONF_KEY = "hbase.regionserver.thrift.compact"; static final String FRAMED_CONF_KEY = "hbase.regionserver.thrift.framed"; - static final String MAX_FRAME_SIZE_CONF_KEY = "hbase.regionserver.thrift.framed.max_frame_size_in_mb"; + static final String MAX_FRAME_SIZE_CONF_KEY = + "hbase.regionserver.thrift.framed.max_frame_size_in_mb"; static final String PORT_CONF_KEY = "hbase.regionserver.thrift.port"; static final String COALESCE_INC_KEY = "hbase.regionserver.thrift.coalesceIncrement"; static final String USE_HTTP_CONF_KEY = "hbase.regionserver.thrift.http"; @@ -347,7 +348,8 @@ public class ThriftServerRunner implements Runnable { doAsEnabled = conf.getBoolean(THRIFT_SUPPORT_PROXYUSER, false); if (doAsEnabled) { if (!conf.getBoolean(USE_HTTP_CONF_KEY, false)) { - LOG.warn("Fail to enable the doAs feature. hbase.regionserver.thrift.http is not configured "); + LOG.warn("Fail to enable the doAs feature. hbase.regionserver.thrift.http is not " + + "configured "); } } if (qop != null) { @@ -433,7 +435,8 @@ public class ThriftServerRunner implements Runnable { httpServer = new Server(threadPool); // Context handler - ServletContextHandler ctxHandler = new ServletContextHandler(httpServer, "/", ServletContextHandler.SESSIONS); + ServletContextHandler ctxHandler = new ServletContextHandler(httpServer, "/", + ServletContextHandler.SESSIONS); ctxHandler.addServlet(new ServletHolder(thriftHttpServlet), "/*"); // set up Jetty and run the embedded server @@ -508,14 +511,7 @@ public class ThriftServerRunner implements Runnable { */ private void setupServer() throws Exception { // Construct correct ProtocolFactory - TProtocolFactory protocolFactory; - if (conf.getBoolean(COMPACT_CONF_KEY, false)) { - LOG.debug("Using compact protocol"); - protocolFactory = new TCompactProtocol.Factory(); - } else { - LOG.debug("Using binary protocol"); - protocolFactory = new TBinaryProtocol.Factory(); - } + TProtocolFactory protocolFactory = getProtocolFactory(); final TProcessor p = new Hbase.Processor<>(handler); ImplType implType = ImplType.getServerImpl(conf); @@ -614,10 +610,8 @@ public class ThriftServerRunner implements Runnable { CallQueue callQueue = new CallQueue(new LinkedBlockingQueue<>(), metrics); ExecutorService executorService = createExecutor( callQueue, serverArgs.getMaxWorkerThreads(), serverArgs.getMaxWorkerThreads()); - serverArgs.executorService(executorService) - .processor(processor) - .transportFactory(transportFactory) - .protocolFactory(protocolFactory); + serverArgs.executorService(executorService).processor(processor) + .transportFactory(transportFactory).protocolFactory(protocolFactory); tserver = new THsHaServer(serverArgs); } else { // THREADED_SELECTOR TThreadedSelectorServer.Args serverArgs = @@ -625,10 +619,8 @@ public class ThriftServerRunner implements Runnable { CallQueue callQueue = new CallQueue(new LinkedBlockingQueue<>(), metrics); ExecutorService executorService = createExecutor( callQueue, serverArgs.getWorkerThreads(), serverArgs.getWorkerThreads()); - serverArgs.executorService(executorService) - .processor(processor) - .transportFactory(transportFactory) - .protocolFactory(protocolFactory); + serverArgs.executorService(executorService).processor(processor) + .transportFactory(transportFactory).protocolFactory(protocolFactory); tserver = new TThreadedSelectorServer(serverArgs); } LOG.info("starting HBase " + implType.simpleClassName() + @@ -640,21 +632,17 @@ public class ThriftServerRunner implements Runnable { THRIFT_SERVER_SOCKET_READ_TIMEOUT_DEFAULT); TServerTransport serverTransport = new TServerSocket( new TServerSocket.ServerSocketTransportArgs(). - bindAddr(new InetSocketAddress(listenAddress, listenPort)). - backlog(backlog). + bindAddr(new InetSocketAddress(listenAddress, listenPort)).backlog(backlog). clientTimeout(readTimeout)); TBoundedThreadPoolServer.Args serverArgs = new TBoundedThreadPoolServer.Args(serverTransport, conf); - serverArgs.processor(processor) - .transportFactory(transportFactory) - .protocolFactory(protocolFactory); + serverArgs.processor(processor).transportFactory(transportFactory) + .protocolFactory(protocolFactory); LOG.info("starting " + ImplType.THREAD_POOL.simpleClassName() + " on " + listenAddress + ":" + Integer.toString(listenPort) + " with readTimeout " + readTimeout + "ms; " + serverArgs); - TBoundedThreadPoolServer tserver = - new TBoundedThreadPoolServer(serverArgs, metrics); - this.tserver = tserver; + this.tserver = new TBoundedThreadPoolServer(serverArgs, metrics); } else { throw new AssertionError("Unsupported Thrift server implementation: " + implType.simpleClassName()); @@ -672,6 +660,20 @@ public class ThriftServerRunner implements Runnable { registerFilters(conf); } + private TProtocolFactory getProtocolFactory() { + TProtocolFactory protocolFactory; + + if (conf.getBoolean(COMPACT_CONF_KEY, false)) { + LOG.debug("Using compact protocol"); + protocolFactory = new TCompactProtocol.Factory(); + } else { + LOG.debug("Using binary protocol"); + protocolFactory = new TBinaryProtocol.Factory(); + } + + return protocolFactory; + } + ExecutorService createExecutor(BlockingQueue<Runnable> callQueue, int minWorkers, int maxWorkers) { ThreadFactoryBuilder tfb = new ThreadFactoryBuilder(); @@ -697,7 +699,7 @@ public class ThriftServerRunner implements Runnable { boolean sortResultColumns) { scanner = resultScanner; sortColumns = sortResultColumns; - } + } public ResultScanner getScanner() { return scanner; @@ -749,10 +751,9 @@ public class ThriftServerRunner implements Runnable { * @param tableName * name of table * @return Table object - * @throws IOException + * @throws IOException if getting the table fails */ - public Table getTable(final byte[] tableName) throws - IOException { + public Table getTable(final byte[] tableName) throws IOException { String table = Bytes.toString(tableName); return connectionCache.getTable(table); } @@ -765,10 +766,10 @@ public class ThriftServerRunner implements Runnable { * Assigns a unique ID to the scanner and adds the mapping to an internal * hash-map. * - * @param scanner + * @param scanner the {@link ResultScanner} to add * @return integer scanner id */ - protected synchronized int addScanner(ResultScanner scanner,boolean sortColumns) { + protected synchronized int addScanner(ResultScanner scanner, boolean sortColumns) { int id = nextScannerId++; ResultScannerWrapper resultScannerWrapper = new ResultScannerWrapper(scanner, sortColumns); scannerMap.put(id, resultScannerWrapper); @@ -778,7 +779,7 @@ public class ThriftServerRunner implements Runnable { /** * Returns the scanner associated with the specified ID. * - * @param id + * @param id the ID of the scanner to get * @return a Scanner, or null if ID was invalid. */ protected synchronized ResultScannerWrapper getScanner(int id) { @@ -789,7 +790,7 @@ public class ThriftServerRunner implements Runnable { * Removes the scanner associated with the specified ID from the internal * id->scanner hash-map. * - * @param id + * @param id the ID of the scanner to remove * @return a Scanner, or null if ID was invalid. */ protected synchronized ResultScannerWrapper removeScanner(int id) { @@ -1116,9 +1117,9 @@ public class ThriftServerRunner implements Runnable { for(ByteBuffer column : columns) { byte [][] famAndQf = CellUtil.parseColumn(getBytes(column)); if (famAndQf.length == 1) { - get.addFamily(famAndQf[0]); + get.addFamily(famAndQf[0]); } else { - get.addColumn(famAndQf[0], famAndQf[1]); + get.addColumn(famAndQf[0], famAndQf[1]); } } get.setTimeRange(0, timestamp); @@ -1361,10 +1362,12 @@ public class ThriftServerRunner implements Runnable { put.setDurability(m.writeToWAL ? Durability.SYNC_WAL : Durability.SKIP_WAL); } } - if (!delete.isEmpty()) + if (!delete.isEmpty()) { table.delete(delete); - if (!put.isEmpty()) + } + if (!put.isEmpty()) { table.put(put); + } } catch (IOException e) { LOG.warn(e.getMessage(), e); throw getIOError(e); @@ -1434,19 +1437,23 @@ public class ThriftServerRunner implements Runnable { put.setDurability(m.writeToWAL ? Durability.SYNC_WAL : Durability.SKIP_WAL); } } - if (!delete.isEmpty()) + if (!delete.isEmpty()) { deletes.add(delete); - if (!put.isEmpty()) + } + if (!put.isEmpty()) { puts.add(put); + } } Table table = null; try { table = getTable(tableName); - if (!puts.isEmpty()) + if (!puts.isEmpty()) { table.put(puts); - if (!deletes.isEmpty()) + } + if (!deletes.isEmpty()) { table.delete(deletes); + } } catch (IOException e) { LOG.warn(e.getMessage(), e); @@ -1762,8 +1769,7 @@ public class ThriftServerRunner implements Runnable { } } - private void closeTable(Table table) throws IOError - { + private void closeTable(Table table) throws IOError { try{ if(table != null){ table.close(); @@ -1885,7 +1891,7 @@ public class ThriftServerRunner implements Runnable { LOG.warn(e.getMessage(), e); throw getIOError(e); } finally{ - closeTable(table); + closeTable(table); } } @@ -1932,7 +1938,7 @@ public class ThriftServerRunner implements Runnable { LOG.warn(e.getMessage(), e); throw new IllegalArgument(Throwables.getStackTraceAsString(e)); } finally { - closeTable(table); + closeTable(table); } } } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftUtilities.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftUtilities.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftUtilities.java index 4865ac3..90f11ad 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftUtilities.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftUtilities.java @@ -46,16 +46,17 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.yetus.audience.InterfaceAudience; @InterfaceAudience.Private -public class ThriftUtilities { +public final class ThriftUtilities { + private ThriftUtilities() { + } /** * This utility method creates a new Hbase HColumnDescriptor object based on a * Thrift ColumnDescriptor "struct". * - * @param in - * Thrift ColumnDescriptor object + * @param in Thrift ColumnDescriptor object * @return HColumnDescriptor - * @throws IllegalArgument + * @throws IllegalArgument if the column name is empty */ static public HColumnDescriptor colDescFromThrift(ColumnDescriptor in) throws IllegalArgument { @@ -150,31 +151,35 @@ public class ThriftUtilities { */ static public List<TRowResult> rowResultFromHBase(Result[] in, boolean sortColumns) { List<TRowResult> results = new ArrayList<>(in.length); - for ( Result result_ : in) { - if(result_ == null || result_.isEmpty()) { - continue; + for (Result result_ : in) { + if(result_ == null || result_.isEmpty()) { + continue; + } + + TRowResult result = new TRowResult(); + result.row = ByteBuffer.wrap(result_.getRow()); + + if (sortColumns) { + result.sortedColumns = new ArrayList<>(); + for (Cell kv : result_.rawCells()) { + result.sortedColumns.add(new TColumn( + ByteBuffer.wrap(CellUtil.makeColumn(CellUtil.cloneFamily(kv), + CellUtil.cloneQualifier(kv))), + new TCell(ByteBuffer.wrap(CellUtil.cloneValue(kv)), kv.getTimestamp()))); } - TRowResult result = new TRowResult(); - result.row = ByteBuffer.wrap(result_.getRow()); - if (sortColumns) { - result.sortedColumns = new ArrayList<>(); - for (Cell kv : result_.rawCells()) { - result.sortedColumns.add(new TColumn( - ByteBuffer.wrap(CellUtil.makeColumn(CellUtil.cloneFamily(kv), - CellUtil.cloneQualifier(kv))), - new TCell(ByteBuffer.wrap(CellUtil.cloneValue(kv)), kv.getTimestamp()))); - } - } else { - result.columns = new TreeMap<>(); - for (Cell kv : result_.rawCells()) { - result.columns.put( - ByteBuffer.wrap(CellUtil.makeColumn(CellUtil.cloneFamily(kv), - CellUtil.cloneQualifier(kv))), - new TCell(ByteBuffer.wrap(CellUtil.cloneValue(kv)), kv.getTimestamp())); - } + } else { + result.columns = new TreeMap<>(); + for (Cell kv : result_.rawCells()) { + result.columns.put( + ByteBuffer.wrap(CellUtil.makeColumn(CellUtil.cloneFamily(kv), + CellUtil.cloneQualifier(kv))), + new TCell(ByteBuffer.wrap(CellUtil.cloneValue(kv)), kv.getTimestamp())); } + } + results.add(result); } + return results; } @@ -204,7 +209,11 @@ public class ThriftUtilities { public static Increment incrementFromThrift(TIncrement tincrement) { Increment inc = new Increment(tincrement.getRow()); byte[][] famAndQf = CellUtil.parseColumn(tincrement.getColumn()); - if (famAndQf.length != 2) return null; + + if (famAndQf.length != 2) { + return null; + } + inc.addColumn(famAndQf[0], famAndQf[1], tincrement.getAmmount()); return inc; } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java index b81c6f4..d98cc50 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftServer.java @@ -89,8 +89,8 @@ import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder; /** - * ThriftServer - this class starts up a Thrift server which implements the HBase API specified in the - * HbaseClient.thrift IDL file. + * ThriftServer - this class starts up a Thrift server which implements the HBase API specified in + * the HbaseClient.thrift IDL file. */ @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.TOOLS) @SuppressWarnings({ "rawtypes", "unchecked" }) @@ -155,11 +155,14 @@ public class ThriftServer extends Configured implements Tool { options.addOption("ro", "readonly", false, "Respond only to read method requests [default: false]"); OptionGroup servers = new OptionGroup(); - servers.addOption( - new Option("nonblocking", false, "Use the TNonblockingServer. This implies the framed transport.")); - servers.addOption(new Option("hsha", false, "Use the THsHaServer. This implies the framed transport.")); - servers.addOption(new Option("selector", false, "Use the TThreadedSelectorServer. This implies the framed transport.")); - servers.addOption(new Option("threadpool", false, "Use the TThreadPoolServer. This is the default.")); + servers.addOption(new Option("nonblocking", false, + "Use the TNonblockingServer. This implies the framed transport.")); + servers.addOption(new Option("hsha", false, + "Use the THsHaServer. This implies the framed transport.")); + servers.addOption(new Option("selector", false, + "Use the TThreadedSelectorServer. This implies the framed transport.")); + servers.addOption(new Option("threadpool", false, + "Use the TThreadPoolServer. This is the default.")); options.addOptionGroup(servers); return options; } @@ -243,8 +246,9 @@ public class ThriftServer extends Configured implements Tool { } } - private static TServer getTNonBlockingServer(TProtocolFactory protocolFactory, TProcessor processor, - TTransportFactory transportFactory, InetSocketAddress inetSocketAddress) throws TTransportException { + private static TServer getTNonBlockingServer(TProtocolFactory protocolFactory, + TProcessor processor, TTransportFactory transportFactory, InetSocketAddress inetSocketAddress) + throws TTransportException { TNonblockingServerTransport serverTransport = new TNonblockingServerSocket(inetSocketAddress); log.info("starting HBase Nonblocking Thrift server on " + inetSocketAddress.toString()); TNonblockingServer.Args serverArgs = new TNonblockingServer.Args(serverTransport); @@ -284,10 +288,10 @@ public class ThriftServer extends Configured implements Tool { log.info("starting HBase ThreadedSelector Thrift server on " + inetSocketAddress.toString()); TThreadedSelectorServer.Args serverArgs = new TThreadedSelectorServer.Args(serverTransport); if (workerThreads > 0) { - serverArgs.workerThreads(workerThreads); + serverArgs.workerThreads(workerThreads); } if (selectorThreads > 0) { - serverArgs.selectorThreads(selectorThreads); + serverArgs.selectorThreads(selectorThreads); } ExecutorService executorService = createExecutor( @@ -378,31 +382,18 @@ public class ThriftServer extends Configured implements Tool { @Override public int run(String[] args) throws Exception { final Configuration conf = getConf(); - TServer server = null; Options options = getOptions(); CommandLine cmd = parseArguments(conf, options, args); int workerThreads = 0; int selectorThreads = 0; int maxCallQueueSize = -1; // use unbounded queue by default - /** - * This is to please both bin/hbase and bin/hbase-daemon. hbase-daemon provides "start" and "stop" arguments hbase - * should print the help if no argument is provided - */ - List<?> argList = cmd.getArgList(); - if (cmd.hasOption("help") || !argList.contains("start") || argList.contains("stop")) { - printUsage(); + if (checkArguments(cmd)) { return 1; } // Get address to bind - String bindAddress; - if (cmd.hasOption("bind")) { - bindAddress = cmd.getOptionValue("bind"); - conf.set("hbase.thrift.info.bindAddress", bindAddress); - } else { - bindAddress = conf.get("hbase.thrift.info.bindAddress"); - } + String bindAddress = getBindAddress(conf, cmd); // check if server should only process read requests, if so override the conf if (cmd.hasOption("readonly")) { @@ -413,35 +404,13 @@ public class ThriftServer extends Configured implements Tool { } // Get read timeout - int readTimeout = THRIFT_SERVER_SOCKET_READ_TIMEOUT_DEFAULT; - if (cmd.hasOption(READ_TIMEOUT_OPTION)) { - try { - readTimeout = Integer.parseInt(cmd.getOptionValue(READ_TIMEOUT_OPTION)); - } catch (NumberFormatException e) { - throw new RuntimeException("Could not parse the value provided for the timeout option", e); - } - } else { - readTimeout = conf.getInt(THRIFT_SERVER_SOCKET_READ_TIMEOUT_KEY, - THRIFT_SERVER_SOCKET_READ_TIMEOUT_DEFAULT); - } - + int readTimeout = getReadTimeout(conf, cmd); // Get port to bind to - int listenPort = 0; - try { - if (cmd.hasOption("port")) { - listenPort = Integer.parseInt(cmd.getOptionValue("port")); - } else { - listenPort = conf.getInt("hbase.regionserver.thrift.port", DEFAULT_LISTEN_PORT); - } - } catch (NumberFormatException e) { - throw new RuntimeException("Could not parse the value provided for the port option", e); - } - + int listenPort = getListenPort(conf, cmd); // Thrift's implementation uses '0' as a placeholder for 'use the default.' int backlog = conf.getInt(BACKLOG_CONF_KEY, 0); - // Local hostname and user name, - // used only if QOP is configured. + // Local hostname and user name, used only if QOP is configured. String host = null; String name = null; @@ -453,8 +422,7 @@ public class ThriftServer extends Configured implements Tool { host = Strings.domainNamePointerToHostName(DNS.getDefaultHost( conf.get("hbase.thrift.dns.interface", "default"), conf.get("hbase.thrift.dns.nameserver", "default"))); - userProvider.login("hbase.thrift.keytab.file", - "hbase.thrift.kerberos.principal", host); + userProvider.login("hbase.thrift.keytab.file", "hbase.thrift.kerberos.principal", host); } UserGroupInformation realUser = userProvider.getCurrent().getUGI(); @@ -463,12 +431,10 @@ public class ThriftServer extends Configured implements Tool { if (stringQop != null) { qop = SaslUtil.getQop(stringQop); if (!securityEnabled) { - throw new IOException("Thrift server must" - + " run in secure mode to support authentication"); + throw new IOException("Thrift server must run in secure mode to support authentication"); } // Extract the name from the principal - name = SecurityUtil.getUserFromPrincipal( - conf.get("hbase.thrift.kerberos.principal")); + name = SecurityUtil.getUserFromPrincipal(conf.get("hbase.thrift.kerberos.principal")); } boolean nonblocking = cmd.hasOption("nonblocking"); @@ -478,14 +444,7 @@ public class ThriftServer extends Configured implements Tool { ThriftMetrics metrics = new ThriftMetrics(conf, ThriftMetrics.ThriftServerType.TWO); final JvmPauseMonitor pauseMonitor = new JvmPauseMonitor(conf, metrics.getSource()); - String implType = "threadpool"; - if (nonblocking) { - implType = "nonblocking"; - } else if (hsha) { - implType = "hsha"; - } else if (selector) { - implType = "selector"; - } + String implType = getImplType(nonblocking, hsha, selector); conf.set("hbase.regionserver.thrift.server.type", implType); conf.setInt("hbase.regionserver.thrift.port", listenPort); @@ -549,49 +508,12 @@ public class ThriftServer extends Configured implements Tool { } // Put up info server. - int port = conf.getInt("hbase.thrift.info.port", 9095); - if (port >= 0) { - conf.setLong("startcode", System.currentTimeMillis()); - String a = conf.get("hbase.thrift.info.bindAddress", "0.0.0.0"); - InfoServer infoServer = new InfoServer("thrift", a, port, false, conf); - infoServer.setAttribute("hbase.conf", conf); - infoServer.start(); - } + startInfoServer(conf); - if (nonblocking) { - server = getTNonBlockingServer(protocolFactory, - processor, - transportFactory, - inetSocketAddress); - } else if (hsha) { - server = getTHsHaServer(protocolFactory, - processor, - transportFactory, - workerThreads, - maxCallQueueSize, - inetSocketAddress, - metrics); - } else if (selector) { - server = getTThreadedSelectorServer(protocolFactory, - processor, - transportFactory, - workerThreads, - selectorThreads, - maxCallQueueSize, - inetSocketAddress, - metrics); - } else { - server = getTThreadPoolServer(protocolFactory, - processor, - transportFactory, - workerThreads, - inetSocketAddress, - backlog, - readTimeout, - metrics); - } + final TServer tserver = getServer(workerThreads, selectorThreads, maxCallQueueSize, readTimeout, + backlog, nonblocking, hsha, selector, metrics, protocolFactory, processor, + transportFactory, inetSocketAddress); - final TServer tserver = server; realUser.doAs( new PrivilegedAction<Object>() { @Override @@ -608,4 +530,106 @@ public class ThriftServer extends Configured implements Tool { // when tserver.stop eventually happens we'll get here. return 0; } + + private String getImplType(boolean nonblocking, boolean hsha, boolean selector) { + String implType = "threadpool"; + + if (nonblocking) { + implType = "nonblocking"; + } else if (hsha) { + implType = "hsha"; + } else if (selector) { + implType = "selector"; + } + + return implType; + } + + private boolean checkArguments(CommandLine cmd) { + /* + * This is to please both bin/hbase and bin/hbase-daemon. hbase-daemon provides "start" and + * "stop" arguments hbase should print the help if no argument is provided + */ + List<?> argList = cmd.getArgList(); + if (cmd.hasOption("help") || !argList.contains("start") || argList.contains("stop")) { + printUsage(); + return true; + } + return false; + } + + private String getBindAddress(Configuration conf, CommandLine cmd) { + String bindAddress; + if (cmd.hasOption("bind")) { + bindAddress = cmd.getOptionValue("bind"); + conf.set("hbase.thrift.info.bindAddress", bindAddress); + } else { + bindAddress = conf.get("hbase.thrift.info.bindAddress"); + } + return bindAddress; + } + + private int getListenPort(Configuration conf, CommandLine cmd) { + int listenPort; + try { + if (cmd.hasOption("port")) { + listenPort = Integer.parseInt(cmd.getOptionValue("port")); + } else { + listenPort = conf.getInt("hbase.regionserver.thrift.port", DEFAULT_LISTEN_PORT); + } + } catch (NumberFormatException e) { + throw new RuntimeException("Could not parse the value provided for the port option", e); + } + return listenPort; + } + + private int getReadTimeout(Configuration conf, CommandLine cmd) { + int readTimeout; + if (cmd.hasOption(READ_TIMEOUT_OPTION)) { + try { + readTimeout = Integer.parseInt(cmd.getOptionValue(READ_TIMEOUT_OPTION)); + } catch (NumberFormatException e) { + throw new RuntimeException("Could not parse the value provided for the timeout option", e); + } + } else { + readTimeout = conf.getInt(THRIFT_SERVER_SOCKET_READ_TIMEOUT_KEY, + THRIFT_SERVER_SOCKET_READ_TIMEOUT_DEFAULT); + } + return readTimeout; + } + + private void startInfoServer(Configuration conf) throws IOException { + int port = conf.getInt("hbase.thrift.info.port", 9095); + + if (port >= 0) { + conf.setLong("startcode", System.currentTimeMillis()); + String a = conf.get("hbase.thrift.info.bindAddress", "0.0.0.0"); + InfoServer infoServer = new InfoServer("thrift", a, port, false, conf); + infoServer.setAttribute("hbase.conf", conf); + infoServer.start(); + } + } + + private TServer getServer(int workerThreads, int selectorThreads, int maxCallQueueSize, + int readTimeout, int backlog, boolean nonblocking, boolean hsha, boolean selector, + ThriftMetrics metrics, TProtocolFactory protocolFactory, TProcessor processor, + TTransportFactory transportFactory, InetSocketAddress inetSocketAddress) + throws TTransportException { + TServer server; + + if (nonblocking) { + server = getTNonBlockingServer(protocolFactory, processor, transportFactory, + inetSocketAddress); + } else if (hsha) { + server = getTHsHaServer(protocolFactory, processor, transportFactory, workerThreads, + maxCallQueueSize, inetSocketAddress, metrics); + } else if (selector) { + server = getTThreadedSelectorServer(protocolFactory, processor, transportFactory, + workerThreads, selectorThreads, maxCallQueueSize, inetSocketAddress, metrics); + } else { + server = getTThreadPoolServer(protocolFactory, processor, transportFactory, workerThreads, + inetSocketAddress, backlog, readTimeout, metrics); + } + return server; + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java index 8ab5a01..88f96cb 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java @@ -74,7 +74,7 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.yetus.audience.InterfaceAudience; @InterfaceAudience.Private -public class ThriftUtilities { +public final class ThriftUtilities { private ThriftUtilities() { throw new UnsupportedOperationException("Can't initialize class"); @@ -287,35 +287,37 @@ public class ThriftUtilities { for (TColumn column : in.getColumns()) { if (in.isSetDeleteType()) { switch (in.getDeleteType()) { - case DELETE_COLUMN: - if (column.isSetTimestamp()) { - out.addColumn(column.getFamily(), column.getQualifier(), column.getTimestamp()); - } else { - out.addColumn(column.getFamily(), column.getQualifier()); - } - break; - case DELETE_COLUMNS: - if (column.isSetTimestamp()) { - out.addColumns(column.getFamily(), column.getQualifier(), column.getTimestamp()); - } else { - out.addColumns(column.getFamily(), column.getQualifier()); - } - break; - case DELETE_FAMILY: - if (column.isSetTimestamp()) { - out.addFamily(column.getFamily(), column.getTimestamp()); - } else { - out.addFamily(column.getFamily()); - } - break; - case DELETE_FAMILY_VERSION: - if (column.isSetTimestamp()) { - out.addFamilyVersion(column.getFamily(), column.getTimestamp()); - } else { - throw new IllegalArgumentException( - "Timestamp is required for TDelete with DeleteFamilyVersion type"); - } - break; + case DELETE_COLUMN: + if (column.isSetTimestamp()) { + out.addColumn(column.getFamily(), column.getQualifier(), column.getTimestamp()); + } else { + out.addColumn(column.getFamily(), column.getQualifier()); + } + break; + case DELETE_COLUMNS: + if (column.isSetTimestamp()) { + out.addColumns(column.getFamily(), column.getQualifier(), column.getTimestamp()); + } else { + out.addColumns(column.getFamily(), column.getQualifier()); + } + break; + case DELETE_FAMILY: + if (column.isSetTimestamp()) { + out.addFamily(column.getFamily(), column.getTimestamp()); + } else { + out.addFamily(column.getFamily()); + } + break; + case DELETE_FAMILY_VERSION: + if (column.isSetTimestamp()) { + out.addFamilyVersion(column.getFamily(), column.getTimestamp()); + } else { + throw new IllegalArgumentException( + "Timestamp is required for TDelete with DeleteFamilyVersion type"); + } + break; + default: + throw new IllegalArgumentException("DeleteType is required for TDelete"); } } else { throw new IllegalArgumentException("DeleteType is required for TDelete"); @@ -416,12 +418,15 @@ public class ThriftUtilities { public static Scan scanFromThrift(TScan in) throws IOException { Scan out = new Scan(); - if (in.isSetStartRow()) + if (in.isSetStartRow()) { out.setStartRow(in.getStartRow()); - if (in.isSetStopRow()) + } + if (in.isSetStopRow()) { out.setStopRow(in.getStopRow()); - if (in.isSetCaching()) + } + if (in.isSetCaching()) { out.setCaching(in.getCaching()); + } if (in.isSetMaxVersions()) { out.setMaxVersions(in.getMaxVersions()); } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestCallQueue.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestCallQueue.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestCallQueue.java index a13774d..435b0da 100644 --- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestCallQueue.java +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestCallQueue.java @@ -109,7 +109,7 @@ public class TestCallQueue { private static void verifyMetrics(ThriftMetrics metrics, String name, int expectValue) throws Exception { - metricsHelper.assertCounter(name, expectValue, metrics.getSource()); + metricsHelper.assertCounter(name, expectValue, metrics.getSource()); } private static Runnable createDummyRunnable() { http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java index 4594ae6..d69da6c 100644 --- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftHttpServer.java @@ -138,7 +138,7 @@ public class TestThriftHttpServer { // wait up to 10s for the server to start for (int i = 0; i < 100 - && ( thriftServer.serverRunner == null || thriftServer.serverRunner.httpServer == + && (thriftServer.serverRunner == null || thriftServer.serverRunner.httpServer == null); i++) { Thread.sleep(100); } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java index 5108eb3..f3e6dbb 100644 --- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java @@ -101,7 +101,7 @@ public class TestThriftServer { private static ByteBuffer valueBname = asByteBuffer("valueB"); private static ByteBuffer valueCname = asByteBuffer("valueC"); private static ByteBuffer valueDname = asByteBuffer("valueD"); - private static ByteBuffer valueEname = asByteBuffer(100l); + private static ByteBuffer valueEname = asByteBuffer(100L); @Rule public TestName name = new TestName(); @@ -124,8 +124,6 @@ public class TestThriftServer { * consolidate all testing to one method because HBaseClusterTestCase * is prone to OutOfMemoryExceptions when there are three or more * JUnit test methods. - * - * @throws Exception */ @Test public void testAll() throws Exception { @@ -147,8 +145,6 @@ public class TestThriftServer { * Tests for creating, enabling, disabling, and deleting tables. Also * tests that creating a table with an invalid column name yields an * IllegalArgument exception. - * - * @throws Exception */ public void doTestTableCreateDrop() throws Exception { ThriftServerRunner.HBaseHandler handler = @@ -181,10 +177,6 @@ public class TestThriftServer { * TODO: These counts are supposed to be zero but sometimes they are not, they are equal to the * passed in maybe. Investigate why. My guess is they are set by the test that runs just * previous to this one. Sometimes they are cleared. Sometimes not. - * @param name - * @param maybe - * @param metrics - * @return */ private int getCurrentCount(final String name, final int maybe, final ThriftMetrics metrics) { int currentCount = 0; @@ -220,12 +212,14 @@ public class TestThriftServer { handler.getTableNames(); // This will have an artificial delay. // 3 to 6 seconds (to account for potential slowness), measured in nanoseconds - try { - metricsHelper.assertGaugeGt("getTableNames_avg_time", 3L * 1000 * 1000 * 1000, metrics.getSource()); - metricsHelper.assertGaugeLt("getTableNames_avg_time",6L * 1000 * 1000 * 1000, metrics.getSource()); - } catch (AssertionError e) { - LOG.info("Fix me! Why does this happen? A concurrent cluster running?", e); - } + try { + metricsHelper.assertGaugeGt("getTableNames_avg_time", 3L * 1000 * 1000 * 1000, + metrics.getSource()); + metricsHelper.assertGaugeLt("getTableNames_avg_time",6L * 1000 * 1000 * 1000, + metrics.getSource()); + } catch (AssertionError e) { + LOG.info("Fix me! Why does this happen? A concurrent cluster running?", e); + } } private static Hbase.Iface getHandlerForMetricsTest(ThriftMetrics metrics, Configuration conf) @@ -235,7 +229,7 @@ public class TestThriftServer { } private static ThriftMetrics getMetrics(Configuration conf) throws Exception { - return new ThriftMetrics( conf, ThriftMetrics.ThriftServerType.ONE); + return new ThriftMetrics(conf, ThriftMetrics.ThriftServerType.ONE); } @@ -300,9 +294,11 @@ public class TestThriftServer { Thread.sleep(1000); long lv = handler.get(tableAname, rowAname, columnAname, null).get(0).value.getLong(); // Wait on all increments being flushed - while (handler.coalescer.getQueueSize() != 0) Threads.sleep(10); - assertEquals((100 + (2 * numIncrements)), lv ); + while (handler.coalescer.getQueueSize() != 0) { + Threads.sleep(10); + } + assertEquals((100 + (2 * numIncrements)), lv); lv = handler.get(tableAname, rowBname, columnAAname, null).get(0).value.getLong(); assertEquals((100 + (3 * 7 * numIncrements)), lv); @@ -315,8 +311,6 @@ public class TestThriftServer { * Tests adding a series of Mutations and BatchMutations, including a * delete mutation. Also tests data retrieval, and getting back multiple * versions. - * - * @throws Exception */ public void doTestTableMutations() throws Exception { ThriftServerRunner.HBaseHandler handler = @@ -393,8 +387,6 @@ public class TestThriftServer { * Similar to testTableMutations(), except Mutations are applied with * specific timestamps and data retrieval uses these timestamps to * extract specific versions of data. - * - * @throws Exception */ public void doTestTableTimestampsAndColumns() throws Exception { // Setup @@ -473,8 +465,6 @@ public class TestThriftServer { /** * Tests the four different scanner-opening methods (with and without * a stoprow, with and without a timestamp). - * - * @throws Exception */ public void doTestTableScanners() throws Exception { // Setup @@ -515,7 +505,8 @@ public class TestThriftServer { closeScanner(scanner1, handler); // Test a scanner on all rows and all columns, with timestamp - int scanner2 = handler.scannerOpenTs(tableAname, rowAname, getColumnList(true, true), time1, null); + int scanner2 = handler.scannerOpenTs(tableAname, rowAname, getColumnList(true, true), time1, + null); TRowResult rowResult2a = handler.scannerGet(scanner2).get(0); assertEquals(rowResult2a.columns.size(), 1); // column A deleted, does not exist. @@ -594,8 +585,6 @@ public class TestThriftServer { /** * For HBASE-2556 * Tests for GetTableRegions - * - * @throws Exception */ public void doTestGetTableRegions() throws Exception { ThriftServerRunner.HBaseHandler handler = @@ -659,8 +648,6 @@ public class TestThriftServer { /** * Appends the value to a cell and checks that the cell value is updated properly. - * - * @throws Exception */ public static void doTestAppend() throws Exception { ThriftServerRunner.HBaseHandler handler = @@ -693,8 +680,6 @@ public class TestThriftServer { /** * Check that checkAndPut fails if the cell does not exist, then put in the cell, then check that * the checkAndPut succeeds. - * - * @throws Exception */ public static void doTestCheckAndPut() throws Exception { ThriftServerRunner.HBaseHandler handler = @@ -791,9 +776,8 @@ public class TestThriftServer { } /** - * * @return a List of ColumnDescriptors for use in creating a table. Has one - * default ColumnDescriptor and one ColumnDescriptor with fewer versions + * default ColumnDescriptor and one ColumnDescriptor with fewer versions */ private static List<ColumnDescriptor> getColumnDescriptors() { ArrayList<ColumnDescriptor> cDescriptors = new ArrayList<>(2); @@ -819,15 +803,20 @@ public class TestThriftServer { */ private List<ByteBuffer> getColumnList(boolean includeA, boolean includeB) { List<ByteBuffer> columnList = new ArrayList<>(); - if (includeA) columnList.add(columnAname); - if (includeB) columnList.add(columnBname); + + if (includeA) { + columnList.add(columnAname); + } + if (includeB) { + columnList.add(columnBname); + } + return columnList; } /** - * * @return a List of Mutations for a row, with columnA having valueA - * and columnB having valueB + * and columnB having valueB */ private static List<Mutation> getMutations() { List<Mutation> mutations = new ArrayList<>(2); @@ -837,12 +826,11 @@ public class TestThriftServer { } /** - * * @return a List of BatchMutations with the following effects: - * (rowA, columnA): delete - * (rowA, columnB): place valueC - * (rowB, columnA): place valueC - * (rowB, columnB): place valueD + * (rowA, columnA): delete + * (rowA, columnB): place valueC + * (rowB, columnA): place valueC + * (rowB, columnB): place valueD */ private static List<BatchMutation> getBatchMutations() { List<BatchMutation> batchMutations = new ArrayList<>(3); @@ -871,7 +859,6 @@ public class TestThriftServer { * * @param scannerId the scanner to close * @param handler the HBaseHandler interfacing to HBase - * @throws Exception */ private void closeScanner( int scannerId, ThriftServerRunner.HBaseHandler handler) throws Exception { http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java index 9cfb0fe..44f843f 100644 --- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServerCmdLine.java @@ -89,8 +89,9 @@ public class TestThriftServerCmdLine { continue; } for (boolean specifyCompact : new boolean[] {false, true}) { - parameters.add(new Object[]{implType, specifyFramed, - specifyBindIP, specifyCompact}); + parameters.add(new Object[] { + implType, specifyFramed, specifyBindIP, specifyCompact + }); } } } http://git-wip-us.apache.org/repos/asf/hbase/blob/83017960/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThriftHBaseServiceHandler.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThriftHBaseServiceHandler.java b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThriftHBaseServiceHandler.java index c07471c..2b6332c 100644 --- a/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThriftHBaseServiceHandler.java +++ b/hbase-thrift/src/test/java/org/apache/hadoop/hbase/thrift2/TestThriftHBaseServiceHandler.java @@ -238,7 +238,7 @@ public class TestThriftHBaseServiceHandler { puts.add(new TPut(wrap(rowName2), columnValues)); handler.putMultiple(table, puts); - List<Boolean> existsResult2 = handler.existsAll(table,gets ); + List<Boolean> existsResult2 = handler.existsAll(table, gets); assertTrue(existsResult2.get(0)); assertTrue(existsResult2.get(1)); @@ -590,8 +590,6 @@ public class TestThriftHBaseServiceHandler { /** * check that checkAndPut fails if the cell does not exist, then put in the cell, then check * that the checkAndPut succeeds. - * - * @throws Exception */ @Test public void testCheckAndPut() throws Exception { @@ -637,8 +635,6 @@ public class TestThriftHBaseServiceHandler { /** * check that checkAndDelete fails if the cell does not exist, then put in the cell, then * check that the checkAndDelete succeeds. - * - * @throws Exception */ @Test public void testCheckAndDelete() throws Exception { @@ -733,8 +729,7 @@ public class TestThriftHBaseServiceHandler { /** * Tests keeping a HBase scanner alive for long periods of time. Each call to getScannerRow() - * should reset the ConnectionCache timeout for the scanner's connection - * @throws Exception + * should reset the ConnectionCache timeout for the scanner's connection. */ @Test public void testLongLivedScan() throws Exception { @@ -1047,7 +1042,11 @@ public class TestThriftHBaseServiceHandler { */ private String pad(int n, byte pad) { String res = Integer.toString(n); - while (res.length() < pad) res = "0" + res; + + while (res.length() < pad) { + res = "0" + res; + } + return res; } @@ -1179,7 +1178,7 @@ public class TestThriftHBaseServiceHandler { assertArrayEquals(("testGetScannerResults" + pad(19 - i, (byte) 2)).getBytes(), results.get(i) .getRow()); } - } + } @Test public void testFilterRegistration() throws Exception { @@ -1213,7 +1212,7 @@ public class TestThriftHBaseServiceHandler { assertTrue(handler.exists(table, get)); metricsHelper.assertCounter("put_num_ops", 1, metrics.getSource()); - metricsHelper.assertCounter( "exists_num_ops", 2, metrics.getSource()); + metricsHelper.assertCounter("exists_num_ops", 2, metrics.getSource()); } private static ThriftMetrics getMetrics(Configuration conf) throws Exception { @@ -1265,7 +1264,7 @@ public class TestThriftHBaseServiceHandler { } private void testExceptionType(THBaseService.Iface handler, ThriftMetrics metrics, - ByteBuffer tTableName, byte[] rowkey, ErrorThrowingGetObserver.ErrorType errorType) { + ByteBuffer tTableName, byte[] rowkey, ErrorThrowingGetObserver.ErrorType errorType) { long preGetCounter = metricsHelper.getCounter("get_num_ops", metrics.getSource()); String exceptionKey = errorType.getMetricName(); long preExceptionCounter = metricsHelper.checkCounterExists(exceptionKey, metrics.getSource()) ? @@ -1381,8 +1380,6 @@ public class TestThriftHBaseServiceHandler { /** * Put valueA to a row, make sure put has happened, then create a mutation object to put valueB * and delete ValueA, then check that the row value is only valueB. - * - * @throws Exception */ @Test public void testMutateRow() throws Exception { @@ -1445,8 +1442,6 @@ public class TestThriftHBaseServiceHandler { * Create TPut, TDelete , TIncrement objects, set durability then call ThriftUtility * functions to get Put , Delete and Increment respectively. Use getDurability to make sure * the returned objects have the appropriate durability setting. - * - * @throws Exception */ @Test public void testDurability() throws Exception {
