[jira] [Commented] (DRILL-5169) Reconsider use of AutoCloseable within Drill

["Julian Hyde (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DRILL-5169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15786120#comment-15786120 ] 

Julian Hyde commented on DRILL-5169:
------------------------------------

I think {{DrillCloseable}} should extend {{AutoCloseable}}. You can override the {{close}} method to have no declared exceptions.

People can then close a {{DrillCloseable}} from within try-with-resources if they like. And you can use utilities such as [Guava's Closer|https://google.github.io/guava/releases/19.0/api/docs/com/google/common/io/Closer.html] or [Calcite's Closer|https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/util/Closer.java].

> Reconsider use of AutoCloseable within Drill
> --------------------------------------------
>
>                 Key: DRILL-5169
>                 URL: https://issues.apache.org/jira/browse/DRILL-5169
>             Project: Apache Drill
>          Issue Type: Improvement
>    Affects Versions: 1.8.0
>            Reporter: Paul Rogers
>            Priority: Minor
>
> Drill has many resources that must be closed: value vectors, threads, operators and on and on. The {{close()}} method may sometimes throw an exception or take a long time. Drill has developed, or borrowed from Guava, many utilities to help manage the close operation.
> Java has two forms of "predefined" closeable interfaces: {{Closeable}} and {{AutoCloseable}}. {{Closeable}} is for I/O resources and thus can throw an {{IOException}}. {{AutoCloseable}} throws no exception, and is integrated into the language for use in try-with-resources blocks. Because {{AutoCloseable}} is intended only for this use, any creation or return of an {{AutoCloseable}} outside of a try-with-resources block produces compiler warnings.
> Neither of the two Java interfaces fit Drill's needs. {{Closeable}} throws a particular exception ({{IOException}}) which Drill seldom throws, but does not throw exceptions that Drill does throw.
> Drill has settled on {{AutoCloseable}}, but few of Drill's resources are limited in life to a single try-with-resources block. The result is either hundreds of resource warnings (which developers learn to ignore), or hundreds of insertions of {{@SuppressWarnings("resource")}} tags, which just clutter the code.
> Note that there is nothing special about either of the Java-provided interfaces. {{Closeable}} is simply a convention to allow easy closing of IO resources such as streams and so on. {{AutoCloseable}} exists for the sole purpose of implementing try-with-resource blocks.
> What we need is a Drill-specific interface that provides a common {{close()}} method which throws only unchecked exceptions, but is not required to be used in try-with-resources. Perhaps call this {{DrillCloseable}}.
> Next, reimplement the various close utilities. For example: {{DrillCloseables.closeAll()}} would close a set of resources, suppressing exceptions, and throwing a single combined exception if any operations fail.
> Then, convert all uses of {{AutoCloseable}} to {{DrillCloseable}}, or at least all that are not used in try-with-resources block. Doing so will eliminate many compiler warnings and or suppress warnings tags. Because Java allows classes to implement multiple interfaces, it is even possible for a class to implement both {{DrillCloseable}} and {{AutoCloseable}} in the rare instance where both are needed.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)