I've actually been looking into this recently. Comments inline.
On Fri, Aug 24, 2012 at 9:23 AM, Jeremy Carroll <[email protected]> wrote:
> I am trying to clarify the contract for this method.
> The documentation is:
>
>
> /** Close the query execution and stop query evaluation as soon as
> convenient.
> * It is important to close query execution objects in order to release
> * resources such as working memory and to stop the query execution.
> * Some storage subsystems require explicit ends of operations and this
> * operation will cause those to be called where necessary.
> * No operations on the query execution or any associated
> * result set are permitted after this call.
> * This method should not be called in parallel with other methods on
> the
> * QueryExecution object.
> */
>
> It is then unclear to me whether:
> a) close MUST be called in all circumstances (i.e. it is part of the
> contract of a QueryExcecution)
> b) close SHOULD be called when you don't read all the results in a result
> set or iterator
Currently it is a). You MUST close the QueryExecution object.
Especially in the case that you are using QueryEngineHTTP (if you do
not close this, then it leaves a connection open to the remote
endpoint, and can end up exhausting the Fuseki thread pool over time).
It does not close automatically after you finish iterating the
ResultSet returned from execSelect() and possibly
execConstructTriples(). I feel like this is a bug. As a back-up for
the developer not closing the QueryExecution, the code should close it
when they've iterated through the entire ResultSet. Tracked in
JENA-305.
>
> If (a) is the only correct code one where there is effectively a finally
> block which calls close even in abnormal termination
Close() should ideally be called in a finally block, especially if
your program is planning on handling the exception and not crashing.
>
> Also can close() be called twice?
There is a bug in Jena 2.7.3 where it cannot be called twice. This is
fixed in JENA-298.
>
> com.hp.hpl.jena.query.QueryExecution.execConstruct()
>
> e.g.
> QueryExecution exec =
> QueryExecutionFactory.create(queryStr);
> try {
> return exec.execConstruct();
> }
> finally {
> exec.close();
> }
>
>
> compared with
>
> QueryExecution exec =
> QueryExecutionFactory.create(queryStr)
> return exec.execConstruct();
>
> or:
> return
> QueryExecutionFactory.create(queryStr).execConstruct();
>
> except in Exception-al circumstances execConstruct() runs the query to
> completion, so any underlying ExtendedIterators run to termination and do
> not need close being called, so maybe close() is redundant in this case. But
> the contract for QueryExecution.close() is not the same as that for
> ClosableIterator.close()
>
> Of course the latter two suggest that somewhere in the implementation(s) of
> execConstruct() there is a finally block to close the underlying iterators
> ....
>
execAsk(), execConstruct(), and execDescribe() do (or if they don't
currently, they should) close the QueryExecution before they return.
In this case you calling close() is redundant, but can't hurt.
-Stephen
