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

Paul Rogers commented on DRILL-5169:
------------------------------------

Thanks Julian. The key issue are the many, many places that Drill creates 
resources that are intended to be passed along through code (within an operator 
or fragment) and closed at some later time. Create and close are not lexically 
related, as in the try-with-resources case, but are more subtly linked 
semantically. In such cases, we either have the many resource warnings or 
warning suppression annotations.

{{AutoCloseable}} is best when used for resources with lifetime related to a 
lexical unit: files, locks, and so on. Drill's value vectors, {{DrillBufs}}, 
resource handles and all the rest have a much more complex lifecycle.

This is why the thought is to allow classes to extend both {{DrillCloseable}} 
and {{AutoCloseable}} in cases where an object lifecycle does match the lexical 
scope of a try-with-resources block.

What have you done in other projects such as Calcite to work around these 
issues?

> 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)

Reply via email to