Repository: phoenix Updated Branches: refs/heads/4.0 45e2891ce -> 89262f2be
PHOENIX-1250 Remove use of Closeables.closeQuietly Remove the use of Guava's Closeables.closeQuietly to allow using Phoenix within a client-side application that has a more recent version of Guava. After this commit, Phoenix can be built against Guava 18.0 (although full integration tests will not work because HBase/Hadoop still rely on an older version of Guava internally). Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/89262f2b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/89262f2b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/89262f2b Branch: refs/heads/4.0 Commit: 89262f2be4c6bf6b63b6e57364141a2a2d5d673e Parents: 45e2891 Author: Gabriel Reid <gabri...@ngdata.com> Authored: Tue Sep 16 11:58:33 2014 +0200 Committer: Gabriel Reid <gabri...@ngdata.com> Committed: Wed Sep 17 09:17:41 2014 +0200 ---------------------------------------------------------------------- phoenix-core/pom.xml | 4 ++ .../apache/phoenix/cache/ServerCacheClient.java | 4 +- .../phoenix/cache/aggcache/SpillFile.java | 2 +- .../phoenix/cache/aggcache/SpillManager.java | 2 +- .../cache/aggcache/SpillableGroupByCache.java | 2 +- .../apache/phoenix/compile/FromCompiler.java | 2 +- .../GroupedAggregateRegionObserver.java | 3 +- .../phoenix/iterate/TableResultIterator.java | 2 +- .../query/ConnectionQueryServicesImpl.java | 7 ++- .../org/apache/phoenix/util/Closeables.java | 46 +++++++++++++++++--- .../apache/phoenix/flume/sink/PhoenixSink.java | 7 +-- pom.xml | 5 +++ 12 files changed, 62 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/pom.xml ---------------------------------------------------------------------- diff --git a/phoenix-core/pom.xml b/phoenix-core/pom.xml index ff3f5d8..0a8b61b 100644 --- a/phoenix-core/pom.xml +++ b/phoenix-core/pom.xml @@ -325,6 +325,10 @@ <groupId>org.apache.commons</groupId> <artifactId>commons-csv</artifactId> </dependency> + <dependency> + <groupId>com.google.code.findbugs</groupId> + <artifactId>jsr305</artifactId> + </dependency> </dependencies> <profiles> http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java b/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java index 301c452..f4be508 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/cache/ServerCacheClient.java @@ -17,8 +17,6 @@ */ package org.apache.phoenix.cache; -import static com.google.common.io.Closeables.closeQuietly; - import java.io.Closeable; import java.io.IOException; import java.sql.SQLException; @@ -319,7 +317,7 @@ public class ServerCacheClient { LOG.warn("Unable to remove hash cache for " + remainingOnServers, lastThrowable); } } finally { - closeQuietly(iterateOverTable); + Closeables.closeQuietly(iterateOverTable); } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillFile.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillFile.java b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillFile.java index 31ad5ce..8dd64d0 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillFile.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillFile.java @@ -28,11 +28,11 @@ import java.nio.channels.FileChannel.MapMode; import java.util.Map; import java.util.UUID; +import org.apache.phoenix.util.Closeables; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.collect.Maps; -import com.google.common.io.Closeables; /** * This class abstracts a SpillFile It is a accessible on a per page basis http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillManager.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillManager.java b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillManager.java index 3f4bf35..2fbea5c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillManager.java @@ -40,12 +40,12 @@ import org.apache.phoenix.schema.KeyValueSchema; import org.apache.phoenix.schema.ValueBitSet; import org.apache.phoenix.schema.tuple.SingleKeyValueTuple; import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.util.Closeables; import org.apache.phoenix.util.KeyValueUtil; import org.apache.phoenix.util.TupleUtil; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; -import com.google.common.io.Closeables; /** * Class servers as an adapter between the in-memory LRU cache and the Spill data structures. It http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillableGroupByCache.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillableGroupByCache.java b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillableGroupByCache.java index 7caa388..ce18cc2 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillableGroupByCache.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/cache/aggcache/SpillableGroupByCache.java @@ -51,11 +51,11 @@ import org.apache.phoenix.expression.aggregator.ServerAggregators; import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr; import org.apache.phoenix.memory.InsufficientMemoryException; import org.apache.phoenix.memory.MemoryManager.MemoryChunk; +import org.apache.phoenix.util.Closeables; import org.apache.phoenix.util.KeyValueUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.io.Closeables; /** * The main entry point is in GroupedAggregateRegionObserver. It instantiates a SpillableGroupByCache and invokes a http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java index dc262ce..8336f3e 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java @@ -67,6 +67,7 @@ import org.apache.phoenix.schema.PTableType; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.TableNotFoundException; import org.apache.phoenix.schema.TableRef; +import org.apache.phoenix.util.Closeables; import org.apache.phoenix.util.SchemaUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -75,7 +76,6 @@ import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; -import com.google.common.io.Closeables; /** * Validates FROM clause and builds a ColumnResolver for resolving column references http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GroupedAggregateRegionObserver.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GroupedAggregateRegionObserver.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GroupedAggregateRegionObserver.java index 54e1eb2..c352e9a 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GroupedAggregateRegionObserver.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GroupedAggregateRegionObserver.java @@ -66,6 +66,7 @@ import org.apache.phoenix.query.QueryConstants; import org.apache.phoenix.schema.PDataType; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.tuple.MultiKeyValueTuple; +import org.apache.phoenix.util.Closeables; import org.apache.phoenix.util.IndexUtil; import org.apache.phoenix.util.KeyValueUtil; import org.apache.phoenix.util.ScanUtil; @@ -74,9 +75,7 @@ import org.apache.phoenix.util.TupleUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.io.Closeables; /** * Region observer that aggregates grouped rows (i.e. SQL query with GROUP BY clause) http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java b/phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java index 756861b..97ff563 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java @@ -24,10 +24,10 @@ import java.util.List; import org.apache.hadoop.hbase.client.HTableInterface; import org.apache.hadoop.hbase.client.Scan; -import com.google.common.io.Closeables; import org.apache.phoenix.compile.StatementContext; import org.apache.phoenix.schema.TableRef; import org.apache.phoenix.schema.tuple.Tuple; +import org.apache.phoenix.util.Closeables; import org.apache.phoenix.util.ServerUtil; http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java index 043c8fd..7e1e0a5 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java @@ -17,7 +17,6 @@ */ package org.apache.phoenix.query; -import static com.google.common.io.Closeables.closeQuietly; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.CYCLE_FLAG; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.LIMIT_REACHED_FLAG; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.MAX_VALUE; @@ -125,6 +124,7 @@ import org.apache.phoenix.schema.SequenceKey; import org.apache.phoenix.schema.TableAlreadyExistsException; import org.apache.phoenix.schema.TableNotFoundException; import org.apache.phoenix.util.ByteUtil; +import org.apache.phoenix.util.Closeables; import org.apache.phoenix.util.ConfigUtil; import org.apache.phoenix.util.MetaDataUtil; import org.apache.phoenix.util.PhoenixContextExecutor; @@ -140,7 +140,6 @@ import com.google.common.base.Throwables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import com.google.common.io.Closeables; import com.google.protobuf.HBaseZeroCopyByteString; @@ -1716,7 +1715,7 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement } catch (IOException e) { throw ServerUtil.parseServerException(e); } finally { - closeQuietly(htable); + Closeables.closeQuietly(htable); } } finally { sequence.getLock().unlock(); @@ -1742,7 +1741,7 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement } catch (IOException e) { throw ServerUtil.parseServerException(e); } finally { - closeQuietly(htable); + Closeables.closeQuietly(htable); } } finally { sequence.getLock().unlock(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-core/src/main/java/org/apache/phoenix/util/Closeables.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/Closeables.java b/phoenix-core/src/main/java/org/apache/phoenix/util/Closeables.java index c4b15dc..3046929 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/Closeables.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/Closeables.java @@ -17,11 +17,16 @@ */ package org.apache.phoenix.util; +import com.google.common.collect.Iterables; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nullable; import java.io.Closeable; import java.io.IOException; -import java.util.*; - -import com.google.common.collect.Iterables; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedList; /** @@ -29,10 +34,38 @@ import com.google.common.collect.Iterables; * */ public class Closeables { + + private static final Logger LOGGER = LoggerFactory.getLogger(Closeables.class); + /** Not constructed */ private Closeables() { } /** + * Close a {@code Closeable}, returning an {@code IOException} if it occurs while closing + * instead of throwing it. This is nearly a clone of the Guava Closeables.closeQuietly method + * which has long since been removed from Guava. + * + * Use of this method should be avoided -- quietly swallowing IOExceptions (particularly on + * Closeables that are being written to) is a code smell. Use of the equivalent method in + * Guava was done for this reason. + * + * @param closeable the Closeable to be closed, can be null + * @return the IOException if one was thrown, otherwise {@code null} + */ + public static IOException closeQuietly(@Nullable Closeable closeable) { + if (closeable == null) { + return null; + } + try { + closeable.close(); + return null; + } catch (IOException e) { + LOGGER.error("Error closing " + closeable, e); + return e; + } + } + + /** * Allows you to close as many of the {@link Closeable}s as possible. * * If any of the close's fail with an IOException, those exception(s) will @@ -48,11 +81,10 @@ public class Closeables { LinkedList<IOException> exceptions = null; for (Closeable closeable : iterable) { - try { - closeable.close(); - } catch (IOException x) { + IOException ioe = closeQuietly(closeable); + if (ioe != null) { if (exceptions == null) exceptions = new LinkedList<IOException>(); - exceptions.add(x); + exceptions.add(ioe); } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/phoenix-flume/src/main/java/org/apache/phoenix/flume/sink/PhoenixSink.java ---------------------------------------------------------------------- diff --git a/phoenix-flume/src/main/java/org/apache/phoenix/flume/sink/PhoenixSink.java b/phoenix-flume/src/main/java/org/apache/phoenix/flume/sink/PhoenixSink.java index efcbef6..f9c929d 100644 --- a/phoenix-flume/src/main/java/org/apache/phoenix/flume/sink/PhoenixSink.java +++ b/phoenix-flume/src/main/java/org/apache/phoenix/flume/sink/PhoenixSink.java @@ -39,7 +39,6 @@ import org.slf4j.LoggerFactory; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; -import com.google.common.base.Stopwatch; import com.google.common.base.Throwables; import com.google.common.collect.Lists; @@ -140,7 +139,7 @@ public final class PhoenixSink extends AbstractSink implements Configurable { Channel channel = getChannel(); Transaction transaction = null; List<Event> events = Lists.newArrayListWithExpectedSize(this.batchSize); - Stopwatch watch = new Stopwatch().start(); + long startTime = System.nanoTime(); try { transaction = channel.getTransaction(); transaction.begin(); @@ -194,7 +193,9 @@ public final class PhoenixSink extends AbstractSink implements Configurable { throw new EventDeliveryException("Failed to persist message", e); } finally { - logger.error(String.format("Time taken to process [%s] events was [%s] seconds",events.size(),watch.stop().elapsedTime(TimeUnit.SECONDS))); + logger.info(String.format("Time taken to process [%s] events was [%s] seconds", + events.size(), + TimeUnit.SECONDS.convert(System.nanoTime() - startTime, TimeUnit.NANOSECONDS))); if( transaction != null ) { transaction.close(); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/89262f2b/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 42184d4..4ffb4ba 100644 --- a/pom.xml +++ b/pom.xml @@ -476,6 +476,11 @@ <scope>compile</scope> </dependency> <dependency> + <groupId>com.google.code.findbugs</groupId> + <artifactId>jsr305</artifactId> + <version>2.0.1</version> + </dependency> + <dependency> <groupId>org.codehaus.jackson</groupId> <artifactId>jackson-jaxrs</artifactId> <version>${jackson.version}</version>
