You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ehedgehog <gi...@git.apache.org> on 2017/04/21 13:43:54 UTC
[GitHub] jena pull request #244: Open iterators reported when cancelling a query duri...
GitHub user ehedgehog opened a pull request:
https://github.com/apache/jena/pull/244
Open iterators reported when cancelling a query during sorting
As per JENA-1320. When cancellations in QueryIterSort and QueryIterTopN occur
at the wrong moment, the source iterators (the ones being consumed by Sort
or TopN) are not closed in the usual way but only by the QueryIteratorCheck
check.
This fix adds additional calls to close() and closeIterator() so that then
usual (non-cancel) paths close the source iterators, and adds extra
handling of the QueryCancelledException so that when it is thrown
through these classes the source iteratots are close(d).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/epimorphics/jena cancel-sort-cj
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/244.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #244
----
commit b289446ef53f1f4f5ef56cebd235bc1eb830d4ec
Author: Chris Dollin <eh...@gmail.com>
Date: 2017-03-14T15:45:33Z
Merge pull request #3 from apache/master
update from upstream
commit 88e6811552cff567a486bfb0863bed8689ae4a49
Author: Chris Dollin <eh...@gmail.com>
Date: 2017-04-04T14:31:57Z
Merge pull request #4 from apache/master
sync from original
commit 61e891752b12e842d888d48780fb324002853eb9
Author: Chris Dollin <eh...@gmail.com>
Date: 2017-04-06T07:42:39Z
Merge pull request #5 from apache/master
synching with apache/jena
commit 6989f2ff591981cccae59789636cb2401070368d
Author: Chris Dollin <eh...@gmail.com>
Date: 2017-04-19T10:46:32Z
Merge pull request #6 from apache/master
update from original
commit 2fe3c281aa55605552e66cae8ac1484072c1944a
Author: Chris Dollin <eh...@googlemail.com>
Date: 2017-04-20T13:53:01Z
Added some tests for close() on QueryIterSort ad QueryIterTopN.
----
---
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.
---
[GitHub] jena pull request #244: Open iterators reported when cancelling a query duri...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/244#discussion_r112718758
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java ---
@@ -89,6 +92,7 @@ public SortedBindingIterator(final QueryIterator qIter) {
// iterator in a try/finally block, and thus will call
// close() themselves.
catch (QueryCancelledException e) {
+ QueryIterSort.this.close();
--- End diff --
There are tabs in various plaxces. Could you follow the convention of using spaces? Display works better. Your tab size != all other tab sizes != Github's handling of tabs.
---
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.
---
[GitHub] jena pull request #244: Open iterators reported when cancelling a query duri...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/244
---
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.
---
[GitHub] jena pull request #244: Open iterators reported when cancelling a query duri...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/244#discussion_r112720299
--- Diff: jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java ---
@@ -302,7 +358,10 @@ public void setCallback(Callback callback)
@Override
public boolean hasNext()
{
- return delegate.hasNext() ;
+ // self-closing
--- End diff --
More tabs.
---
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.
---
[GitHub] jena pull request #244: Open iterators reported when cancelling a query duri...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/244#discussion_r112720371
--- Diff: jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java ---
@@ -135,6 +133,37 @@ public void testCleanAfterClose()
assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)) ;
}
+ @Test public void testCloseClosesSourceIterator() {
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null, (DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ qis.close();
+ assertTrue("source iterator should have been closed", iterator.isClosed());
+ }
+
+ @Test public void testExhaustionClosesSourceIterator() {
+ iterator.setCallback(new Callback()
--- End diff --
This can be written as `iterator.setCallback(()->{});`
---
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.
---
Re: [] jena issue #244: Open iterators reported when cancelling a
query during sort...
Posted by Andy Seaborne <an...@apache.org>.
On 25/04/17 09:31, ehedgehog wrote:
> Github user ehedgehog commented on the issue:
>
> https://github.com/apache/jena/pull/244
>
> On 24 April 2017 at 12:54, Andy Seaborne <no...@github.com> wrote:
>
> > *@afs* approved this pull request.
> >
>
> Thanks Andy. What's the next step -- or are we done
> for the "open iterators" update?
>
> Chris
>
>
Put into the 3.3.0 release (see other email).
Andy
[GitHub] jena issue #244: Open iterators reported when cancelling a query during sort...
Posted by ehedgehog <gi...@git.apache.org>.
Github user ehedgehog commented on the issue:
https://github.com/apache/jena/pull/244
On 24 April 2017 at 12:54, Andy Seaborne <no...@github.com> wrote:
> *@afs* approved this pull request.
>
Thanks Andy. What's the next step -- or are we done
for the "open iterators" update?
Chris
--
Chris "allusive" Dollin
---
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.
---
Re: [GitHub] jena issue #244: Open iterators reported when cancelling
a query during sort...
Posted by Chris Dollin <ch...@epimorphics.com>.
On 21 April 2017 at 16:54, afs <gi...@git.apache.org> wrote:
> Github user afs commented on the issue:
>
> https://github.com/apache/jena/pull/244
>
> If you "git rebase" a branch against apache/master, not pull/merge
> then you won't get the additional merge commits.
>
>
Thanks for the tip. I'd wondered how to suppress those.
Chris
[GitHub] jena issue #244: Open iterators reported when cancelling a query during sort...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the issue:
https://github.com/apache/jena/pull/244
If you "git rebase" a branch against apache/master, not pull/merge then you won't get the additional merge commits.
---
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.
---