[GitHub] incubator-phoenix pull request: PHOENIX-85 Refine error handling i...

[JamesRTaylor <gi...@git.apache.org>]

Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/incubator-phoenix/pull/10#discussion_r10185708
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/cache/TenantCacheImpl.java ---
    @@ -82,9 +82,17 @@ public Closeable getServerCache(ImmutableBytesPtr cacheId) {
         @Override
         public Closeable addServerCache(ImmutableBytesPtr cacheId, ImmutableBytesWritable cachePtr, ServerCacheFactory cacheFactory) throws SQLException {
             MemoryChunk chunk = this.getMemoryManager().allocate(cachePtr.getLength());
    -        Closeable element = cacheFactory.newCache(cachePtr, chunk);
    -        getServerCaches().put(cacheId, element);
    -        return element;
    +        try {
    --- End diff --
    
    Good catch, but I'd code this like this instead so that you guarantee to close the chunk and ignore any exception that occurs when you close it, as you'd be in the middle of throwing another, different exception:
    
        boolean success = false;
        try {
            Closeable element = cacheFactory.newCache(cachePtr, chunk);
            getServerCaches().put(cacheId, element);
            success = true;
            return element;
        } finally {
            if (!success) Closeables.closeQuietly(chunk); // com.google.common.io.Closeables
        }


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---