Skip to content

Address Jena-1229, fuseki timeout queries not working properly with DISTINCT.#166

Merged
asfgit merged 5 commits intoapache:masterfrom
epimorphics:master
Sep 5, 2016
Merged

Address Jena-1229, fuseki timeout queries not working properly with DISTINCT.#166
asfgit merged 5 commits intoapache:masterfrom
epimorphics:master

Conversation

@ehedgehog
Copy link
Copy Markdown
Contributor

Remove incorrect call super.close() from the body of QueryIterDistinct.requestSubCancel()
as per JENA #1229.

ehedgehog and others added 4 commits July 27, 2016 14:47
- that cancelling a QID cancels its base iterator
- likewise if we're serialising to disc
- closing an unbagged QID doesn't make a bag
- closing a bagged QID closes the bag
@afs
Copy link
Copy Markdown
Member

afs commented Sep 2, 2016

You're lucky!

The additional first commits c88e008 and 18d28f5 don't contaminate the diff.

@ehedgehog
Copy link
Copy Markdown
Contributor Author

Did I manage to mess up commits somehow? That must be because of moving between
repos at home and at work. Sorry.

Is there something I could/should do at my end?

@afs
Copy link
Copy Markdown
Member

afs commented Sep 2, 2016

In the list of commits above, the first commit says "ehedgehog committed on GitHub on 27 Jul" then a merge from jena/master 2 days ago, then the JENA-1229 change. Just a guess: you created a branch, then merged jena/master, rather then getting your local master up to date then branching. But it seems github works it out because if you go to the "166.diff" URL, then there is a short diff, no merge items.

@ehedgehog
Copy link
Copy Markdown
Contributor Author

(Moving from mailing list)

Andy had said:

(/me not entirely convinced that 'close' is always called for all
points where cancel triggers everywhere)

Yes. I've just had a look over some of the code and it
certainly wasn't obvious in that look that the query
ends up closing the result iterator. I could have missed
something easily. But if there's a problem there its
more general than the issue with QueryInterDistinct and
deserves its own JIRA?

But in any case cancel() is [typically] called from the
timeout thread, so it would surely be thread-unsafe for
the cancel methods to go calling close() while the query
thread might be working on shared state, independently
of worries about close() always being called.

[A look at the codebase shows that there are three
classes with requestSubCancel() closing, not cancelling,
other iterators:

AnstractIterHashJoin
QueryIterDistinctReduced
QueryIter2LoopOnLeft

which suggests these should be looked at in case there
might be a similar cancellation problem]

How should we make progress on this?

Chris

@afs
Copy link
Copy Markdown
Member

afs commented Sep 5, 2016

I have fixed those 3 requestSubCancel instances.

@asfgit asfgit merged commit bcb20c6 into apache:master Sep 5, 2016
asfgit pushed a commit that referenced this pull request Sep 5, 2016
@ehedgehog
Copy link
Copy Markdown
Contributor Author

Thanks very much, Andy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants