You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by Andy Seaborne <an...@apache.org> on 2017/04/03 10:15:37 UTC
Re: Bug in SortedDataBag?
On 31/03/17 16:40, Chris Dollin wrote:
> Hi Andy
>
> Yes, the cancel thread doesn't call open(), I understand
> that.
is that "close()" not "open()"?
> The warnings are intermittent because the iterator
> in question (the "source" interator of QueryIterSort)
> is only left open if the cancel request is noticed while
> the sort databag is filling up.
>
> QueryIterSort never explicitly closes the source
> iterator. But if the sort databag pulls all of the
> source bindings, the source iterator self-closes.
> So there's a window while the bag is filling when
> a detected cancellation will leave the source
> iterator open.
>
> A fix is to ensure that the source iterator
> is closed when QueryIterSort closes:
>
> @Override public void closeIterator() {
and also:
this.db.close();
c.f. requestCancel
> embeddedIterator.close();
> super.closeIterator();
> }
>
> "embeddedIterator" is the source iterator.
SortedDataBag: add close() to cancel:
public void cancel() {
comparator.cancel();
close();
}
>
> This fix eliminates the presenting problem and
> the tests continue to pass /except/ that
> CallBackIterator's close() method needs to
> be made a no-op rather than failing, since
> that open() is now actually called.
close()?
>
> [It looks like QueryIterTopN has the same problem]
It would do no harm to have the same close processing in QueryIterTopN
though it does not use data bags because it is an unconditional single
pass over the data.
Have you seen warnings in the TopN case?
Andy
>
> Chris
>
>
>
> On 20 March 2017 at 15:48, Chris Dollin <ch...@epimorphics.com>
> wrote:
>
>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>
>>>
>>> SortedDataBag.cancel does not call close.
>>>
>>
>> You're quite right, am having another look at the problem.
>>
>> Chris
>>
>
>
>
Re: Bug in SortedDataBag?
Posted by Andy Seaborne <an...@apache.org>.
On 04/04/17 09:37, Chris Dollin wrote:
> I ran a simplified version of the test code, with no
> sort but with a limit, and could not produce warnings.
> So I'd not do anything to change TopN unless/until we
> have more warnings to work with.
Let's fix it anyway. Trust the code analysis.
>
> Have we stabilised enough that I turn this into a JIRA/PR with
> a unit test?
Yes.
Andy
>
> Chris
>
>
> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com> wrote:
>
>> Argh, yes, all occurences of "open()" are supposed
>> to be "close()".
>>
>> | Have you seen warnings in the TopN case?
>>
>> I haven't got an isolated TopN test case but it has a similar
>> pattern of keeping references to the source iterator but
>> not closing via them.
>>
>> I can construct an example and see what happens.
>>
>> Chris
>>
>>
>>
>>
>>
>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>
>>>
>>>
>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>
>>>> Hi Andy
>>>>
>>>> Yes, the cancel thread doesn't call open(), I understand
>>>> that.
>>>>
>>>
>>> is that "close()" not "open()"?
>>>
>>> The warnings are intermittent because the iterator
>>>> in question (the "source" interator of QueryIterSort)
>>>> is only left open if the cancel request is noticed while
>>>> the sort databag is filling up.
>>>>
>>>> QueryIterSort never explicitly closes the source
>>>> iterator. But if the sort databag pulls all of the
>>>> source bindings, the source iterator self-closes.
>>>> So there's a window while the bag is filling when
>>>> a detected cancellation will leave the source
>>>> iterator open.
>>>>
>>>> A fix is to ensure that the source iterator
>>>> is closed when QueryIterSort closes:
>>>>
>>>> @Override public void closeIterator() {
>>>>
>>>
>>> and also:
>>>
>>> this.db.close();
>>>
>>> c.f. requestCancel
>>>
>>> embeddedIterator.close();
>>>> super.closeIterator();
>>>> }
>>>>
>>>> "embeddedIterator" is the source iterator.
>>>>
>>>
>>> SortedDataBag: add close() to cancel:
>>>
>>> public void cancel() {
>>> comparator.cancel();
>>> close();
>>> }
>>>
>>>
>>>
>>>> This fix eliminates the presenting problem and
>>>> the tests continue to pass /except/ that
>>>> CallBackIterator's close() method needs to
>>>> be made a no-op rather than failing, since
>>>> that open() is now actually called.
>>>>
>>>
>>> close()?
>>>
>>>
>>>> [It looks like QueryIterTopN has the same problem]
>>>>
>>>
>>> It would do no harm to have the same close processing in QueryIterTopN
>>> though it does not use data bags because it is an unconditional single pass
>>> over the data.
>>>
>>> Have you seen warnings in the TopN case?
>>>
>>> Andy
>>>
>>>
>>>
>>>> Chris
>>>>
>>>>
>>>>
>>>> On 20 March 2017 at 15:48, Chris Dollin <ch...@epimorphics.com>
>>>> wrote:
>>>>
>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>
>>>>>
>>>>>> SortedDataBag.cancel does not call close.
>>>>>>
>>>>>>
>>>>> You're quite right, am having another look at the problem.
>>>>>
>>>>> Chris
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>
>>
>> --
>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>> Affair/
>>
>> Epimorphics Ltd, http://www.epimorphics.com
>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
>> 6PT
>> Epimorphics Ltd. is a limited company registered in England (number
>> 7016688)
>>
>
>
>
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
Query:
PREFIX xsd: <http://www.w3.org/2001/XMLSchema#>
PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
PREFIX rdfs: <http://www.w3.org/2000/01/rdf-schema#>
PREFIX owl: <http://www.w3.org/2002/07/owl#>
PREFIX lrppi: <http://landregistry.data.gov.uk/def/ppi/>
PREFIX skos: <http://www.w3.org/2004/02/skos/core#>
PREFIX lrcommon: <http://landregistry.data.gov.uk/def/common/>
PREFIX ppi: <http://landregistry.data.gov.uk/def/ppi/>
SELECT ?paon ?date {
?trans lrppi:transactionId ?transId ;
lrppi:transactionDate ?date ;
lrppi:propertyAddress ?addr.
OPTIONAL { ?addr lrcommon:paon ?paon }
} ORDER BY DESC(?date)
LIMIT 2 # 00
Chris
On 4 April 2017 at 13:48, Andy Seaborne <an...@apache.org> wrote:
>
>
> On 04/04/17 13:43, Chris Dollin wrote:
>
>> On 4 April 2017 at 13:39, Chris Dollin <ch...@epimorphics.com>
>> wrote:
>>
>>
>> On 4 April 2017 at 09:37, Chris Dollin <ch...@epimorphics.com>
>>>>
>>>>> wrote:
>>>>>
>>>>> I ran a simplified version of the test code, with no
>>>>>> sort but with a limit, and could not produce warnings.
>>>>>>
>>>>>>
>>>>> Well of course not, it needs sort + small limit to involve TopN
>>>>
>>>>
>>>> So. Testing with a version of Fuseki that has the fix for
>> QueryIterSort included, with the query having LIMIT 2 to
>> have TopN used, we get the same (infrequent) warning
>> messages.
>>
>> Making the same change to TopN as we made to Sort and
>> the problem goes away.
>>
>
> What's the query?
>
>
>
>>
>>
>>>>
>>>> So I'd not do anything to change TopN unless/until we
>>>>>
>>>>>> have more warnings to work with.
>>>>>>
>>>>>> Have we stabilised enough that I turn this into a JIRA/PR with
>>>>>> a unit test?
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com>
>>>>>> wrote:
>>>>>>
>>>>>> Argh, yes, all occurences of "open()" are supposed
>>>>>>> to be "close()".
>>>>>>>
>>>>>>> | Have you seen warnings in the TopN case?
>>>>>>>
>>>>>>> I haven't got an isolated TopN test case but it has a similar
>>>>>>> pattern of keeping references to the source iterator but
>>>>>>> not closing via them.
>>>>>>>
>>>>>>> I can construct an example and see what happens.
>>>>>>>
>>>>>>> Chris
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>>>>>>
>>>>>>>> Hi Andy
>>>>>>>>>
>>>>>>>>> Yes, the cancel thread doesn't call open(), I understand
>>>>>>>>> that.
>>>>>>>>>
>>>>>>>>>
>>>>>>>> is that "close()" not "open()"?
>>>>>>>>
>>>>>>>> The warnings are intermittent because the iterator
>>>>>>>>
>>>>>>>>> in question (the "source" interator of QueryIterSort)
>>>>>>>>> is only left open if the cancel request is noticed while
>>>>>>>>> the sort databag is filling up.
>>>>>>>>>
>>>>>>>>> QueryIterSort never explicitly closes the source
>>>>>>>>> iterator. But if the sort databag pulls all of the
>>>>>>>>> source bindings, the source iterator self-closes.
>>>>>>>>> So there's a window while the bag is filling when
>>>>>>>>> a detected cancellation will leave the source
>>>>>>>>> iterator open.
>>>>>>>>>
>>>>>>>>> A fix is to ensure that the source iterator
>>>>>>>>> is closed when QueryIterSort closes:
>>>>>>>>>
>>>>>>>>> @Override public void closeIterator() {
>>>>>>>>>
>>>>>>>>>
>>>>>>>> and also:
>>>>>>>>
>>>>>>>> this.db.close();
>>>>>>>>
>>>>>>>> c.f. requestCancel
>>>>>>>>
>>>>>>>> embeddedIterator.close();
>>>>>>>>
>>>>>>>>> super.closeIterator();
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> "embeddedIterator" is the source iterator.
>>>>>>>>>
>>>>>>>>>
>>>>>>>> SortedDataBag: add close() to cancel:
>>>>>>>>
>>>>>>>> public void cancel() {
>>>>>>>> comparator.cancel();
>>>>>>>> close();
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> This fix eliminates the presenting problem and
>>>>>>>>> the tests continue to pass /except/ that
>>>>>>>>> CallBackIterator's close() method needs to
>>>>>>>>> be made a no-op rather than failing, since
>>>>>>>>> that open() is now actually called.
>>>>>>>>>
>>>>>>>>>
>>>>>>>> close()?
>>>>>>>>
>>>>>>>>
>>>>>>>> [It looks like QueryIterTopN has the same problem]
>>>>>>>>>
>>>>>>>>>
>>>>>>>> It would do no harm to have the same close processing in
>>>>>>>> QueryIterTopN though it does not use data bags because it is an
>>>>>>>> unconditional single pass over the data.
>>>>>>>>
>>>>>>>> Have you seen warnings in the TopN case?
>>>>>>>>
>>>>>>>> Andy
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Chris
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 20 March 2017 at 15:48, Chris Dollin <
>>>>>>>>> chris.dollin@epimorphics.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> SortedDataBag.cancel does not call close.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> You're quite right, am having another look at the problem.
>>>>>>>>>>
>>>>>>>>>> Chris
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> "What I don't understand is this ..." Trevor Chaplin, /The
>>>>>>> Beiderbeck Affair/
>>>>>>>
>>>>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>>>>> BS20 6PT
>>>>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>>>>> 7016688)
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> "What I don't understand is this ..." Trevor Chaplin, /The
>>>>>> Beiderbeck
>>>>>> Affair/
>>>>>>
>>>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>>>> BS20 6PT
>>>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>>>> 7016688)
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>>>> Affair/
>>>>>
>>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>>> BS20 6PT
>>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>>> 7016688)
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>>> Affair/
>>>>
>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>> BS20 6PT
>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>> 7016688)
>>>>
>>>>
>>>
>>>
>>> --
>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>> Affair/
>>>
>>> Epimorphics Ltd, http://www.epimorphics.com
>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>> BS20
>>> 6PT
>>> Epimorphics Ltd. is a limited company registered in England (number
>>> 7016688)
>>>
>>>
>>
>>
>>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)
Re: Bug in SortedDataBag?
Posted by Andy Seaborne <an...@apache.org>.
On 04/04/17 13:43, Chris Dollin wrote:
> On 4 April 2017 at 13:39, Chris Dollin <ch...@epimorphics.com> wrote:
>
>
>>> On 4 April 2017 at 09:37, Chris Dollin <ch...@epimorphics.com>
>>>> wrote:
>>>>
>>>>> I ran a simplified version of the test code, with no
>>>>> sort but with a limit, and could not produce warnings.
>>>>>
>>>>
>>> Well of course not, it needs sort + small limit to involve TopN
>>>
>>>
> So. Testing with a version of Fuseki that has the fix for
> QueryIterSort included, with the query having LIMIT 2 to
> have TopN used, we get the same (infrequent) warning
> messages.
>
> Making the same change to TopN as we made to Sort and
> the problem goes away.
What's the query?
>
>
>>>
>>>
>>>> So I'd not do anything to change TopN unless/until we
>>>>> have more warnings to work with.
>>>>>
>>>>> Have we stabilised enough that I turn this into a JIRA/PR with
>>>>> a unit test?
>>>>>
>>>>> Chris
>>>>>
>>>>>
>>>>> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com>
>>>>> wrote:
>>>>>
>>>>>> Argh, yes, all occurences of "open()" are supposed
>>>>>> to be "close()".
>>>>>>
>>>>>> | Have you seen warnings in the TopN case?
>>>>>>
>>>>>> I haven't got an isolated TopN test case but it has a similar
>>>>>> pattern of keeping references to the source iterator but
>>>>>> not closing via them.
>>>>>>
>>>>>> I can construct an example and see what happens.
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>>>>>
>>>>>>>> Hi Andy
>>>>>>>>
>>>>>>>> Yes, the cancel thread doesn't call open(), I understand
>>>>>>>> that.
>>>>>>>>
>>>>>>>
>>>>>>> is that "close()" not "open()"?
>>>>>>>
>>>>>>> The warnings are intermittent because the iterator
>>>>>>>> in question (the "source" interator of QueryIterSort)
>>>>>>>> is only left open if the cancel request is noticed while
>>>>>>>> the sort databag is filling up.
>>>>>>>>
>>>>>>>> QueryIterSort never explicitly closes the source
>>>>>>>> iterator. But if the sort databag pulls all of the
>>>>>>>> source bindings, the source iterator self-closes.
>>>>>>>> So there's a window while the bag is filling when
>>>>>>>> a detected cancellation will leave the source
>>>>>>>> iterator open.
>>>>>>>>
>>>>>>>> A fix is to ensure that the source iterator
>>>>>>>> is closed when QueryIterSort closes:
>>>>>>>>
>>>>>>>> @Override public void closeIterator() {
>>>>>>>>
>>>>>>>
>>>>>>> and also:
>>>>>>>
>>>>>>> this.db.close();
>>>>>>>
>>>>>>> c.f. requestCancel
>>>>>>>
>>>>>>> embeddedIterator.close();
>>>>>>>> super.closeIterator();
>>>>>>>> }
>>>>>>>>
>>>>>>>> "embeddedIterator" is the source iterator.
>>>>>>>>
>>>>>>>
>>>>>>> SortedDataBag: add close() to cancel:
>>>>>>>
>>>>>>> public void cancel() {
>>>>>>> comparator.cancel();
>>>>>>> close();
>>>>>>> }
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> This fix eliminates the presenting problem and
>>>>>>>> the tests continue to pass /except/ that
>>>>>>>> CallBackIterator's close() method needs to
>>>>>>>> be made a no-op rather than failing, since
>>>>>>>> that open() is now actually called.
>>>>>>>>
>>>>>>>
>>>>>>> close()?
>>>>>>>
>>>>>>>
>>>>>>>> [It looks like QueryIterTopN has the same problem]
>>>>>>>>
>>>>>>>
>>>>>>> It would do no harm to have the same close processing in
>>>>>>> QueryIterTopN though it does not use data bags because it is an
>>>>>>> unconditional single pass over the data.
>>>>>>>
>>>>>>> Have you seen warnings in the TopN case?
>>>>>>>
>>>>>>> Andy
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Chris
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 20 March 2017 at 15:48, Chris Dollin <
>>>>>>>> chris.dollin@epimorphics.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> SortedDataBag.cancel does not call close.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> You're quite right, am having another look at the problem.
>>>>>>>>>
>>>>>>>>> Chris
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> "What I don't understand is this ..." Trevor Chaplin, /The
>>>>>> Beiderbeck Affair/
>>>>>>
>>>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>>>> BS20 6PT
>>>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>>>> 7016688)
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>>>> Affair/
>>>>>
>>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>>> BS20 6PT
>>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>>> 7016688)
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>>> Affair/
>>>>
>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>> BS20 6PT
>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>> 7016688)
>>>>
>>>
>>>
>>>
>>> --
>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>> Affair/
>>>
>>> Epimorphics Ltd, http://www.epimorphics.com
>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>> BS20 6PT
>>> Epimorphics Ltd. is a limited company registered in England (number
>>> 7016688)
>>>
>>
>>
>>
>> --
>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>> Affair/
>>
>> Epimorphics Ltd, http://www.epimorphics.com
>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
>> 6PT
>> Epimorphics Ltd. is a limited company registered in England (number
>> 7016688)
>>
>
>
>
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
On 4 April 2017 at 13:39, Chris Dollin <ch...@epimorphics.com> wrote:
>> On 4 April 2017 at 09:37, Chris Dollin <ch...@epimorphics.com>
>>> wrote:
>>>
>>>> I ran a simplified version of the test code, with no
>>>> sort but with a limit, and could not produce warnings.
>>>>
>>>
>> Well of course not, it needs sort + small limit to involve TopN
>>
>>
So. Testing with a version of Fuseki that has the fix for
QueryIterSort included, with the query having LIMIT 2 to
have TopN used, we get the same (infrequent) warning
messages.
Making the same change to TopN as we made to Sort and
the problem goes away.
>>
>>
>>> So I'd not do anything to change TopN unless/until we
>>>> have more warnings to work with.
>>>>
>>>> Have we stabilised enough that I turn this into a JIRA/PR with
>>>> a unit test?
>>>>
>>>> Chris
>>>>
>>>>
>>>> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com>
>>>> wrote:
>>>>
>>>>> Argh, yes, all occurences of "open()" are supposed
>>>>> to be "close()".
>>>>>
>>>>> | Have you seen warnings in the TopN case?
>>>>>
>>>>> I haven't got an isolated TopN test case but it has a similar
>>>>> pattern of keeping references to the source iterator but
>>>>> not closing via them.
>>>>>
>>>>> I can construct an example and see what happens.
>>>>>
>>>>> Chris
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>>>>
>>>>>>> Hi Andy
>>>>>>>
>>>>>>> Yes, the cancel thread doesn't call open(), I understand
>>>>>>> that.
>>>>>>>
>>>>>>
>>>>>> is that "close()" not "open()"?
>>>>>>
>>>>>> The warnings are intermittent because the iterator
>>>>>>> in question (the "source" interator of QueryIterSort)
>>>>>>> is only left open if the cancel request is noticed while
>>>>>>> the sort databag is filling up.
>>>>>>>
>>>>>>> QueryIterSort never explicitly closes the source
>>>>>>> iterator. But if the sort databag pulls all of the
>>>>>>> source bindings, the source iterator self-closes.
>>>>>>> So there's a window while the bag is filling when
>>>>>>> a detected cancellation will leave the source
>>>>>>> iterator open.
>>>>>>>
>>>>>>> A fix is to ensure that the source iterator
>>>>>>> is closed when QueryIterSort closes:
>>>>>>>
>>>>>>> @Override public void closeIterator() {
>>>>>>>
>>>>>>
>>>>>> and also:
>>>>>>
>>>>>> this.db.close();
>>>>>>
>>>>>> c.f. requestCancel
>>>>>>
>>>>>> embeddedIterator.close();
>>>>>>> super.closeIterator();
>>>>>>> }
>>>>>>>
>>>>>>> "embeddedIterator" is the source iterator.
>>>>>>>
>>>>>>
>>>>>> SortedDataBag: add close() to cancel:
>>>>>>
>>>>>> public void cancel() {
>>>>>> comparator.cancel();
>>>>>> close();
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>>> This fix eliminates the presenting problem and
>>>>>>> the tests continue to pass /except/ that
>>>>>>> CallBackIterator's close() method needs to
>>>>>>> be made a no-op rather than failing, since
>>>>>>> that open() is now actually called.
>>>>>>>
>>>>>>
>>>>>> close()?
>>>>>>
>>>>>>
>>>>>>> [It looks like QueryIterTopN has the same problem]
>>>>>>>
>>>>>>
>>>>>> It would do no harm to have the same close processing in
>>>>>> QueryIterTopN though it does not use data bags because it is an
>>>>>> unconditional single pass over the data.
>>>>>>
>>>>>> Have you seen warnings in the TopN case?
>>>>>>
>>>>>> Andy
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Chris
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 20 March 2017 at 15:48, Chris Dollin <
>>>>>>> chris.dollin@epimorphics.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> SortedDataBag.cancel does not call close.
>>>>>>>>>
>>>>>>>>>
>>>>>>>> You're quite right, am having another look at the problem.
>>>>>>>>
>>>>>>>> Chris
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> "What I don't understand is this ..." Trevor Chaplin, /The
>>>>> Beiderbeck Affair/
>>>>>
>>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>>> BS20 6PT
>>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>>> 7016688)
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>>> Affair/
>>>>
>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>> BS20 6PT
>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>> 7016688)
>>>>
>>>
>>>
>>>
>>> --
>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>> Affair/
>>>
>>> Epimorphics Ltd, http://www.epimorphics.com
>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>> BS20 6PT
>>> Epimorphics Ltd. is a limited company registered in England (number
>>> 7016688)
>>>
>>
>>
>>
>> --
>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>> Affair/
>>
>> Epimorphics Ltd, http://www.epimorphics.com
>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>> BS20 6PT
>> Epimorphics Ltd. is a limited company registered in England (number
>> 7016688)
>>
>
>
>
> --
> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
> Affair/
>
> Epimorphics Ltd, http://www.epimorphics.com
> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
> 6PT
> Epimorphics Ltd. is a limited company registered in England (number
> 7016688)
>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
On 4 April 2017 at 12:44, Chris Dollin <ch...@epimorphics.com> wrote:
> Whoops, send with no content -- fumble-fingered.
>
>
> On 4 April 2017 at 12:43, Chris Dollin <ch...@epimorphics.com>
> wrote:
>
>>
>>
>> On 4 April 2017 at 09:37, Chris Dollin <ch...@epimorphics.com>
>> wrote:
>>
>>> I ran a simplified version of the test code, with no
>>> sort but with a limit, and could not produce warnings.
>>>
>>
> Well of course not, it needs sort + small limit to involve TopN.
>
> Chris
>
>
>
>
>
>> So I'd not do anything to change TopN unless/until we
>>> have more warnings to work with.
>>>
>>> Have we stabilised enough that I turn this into a JIRA/PR with
>>> a unit test?
>>>
>>> Chris
>>>
>>>
>>> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com>
>>> wrote:
>>>
>>>> Argh, yes, all occurences of "open()" are supposed
>>>> to be "close()".
>>>>
>>>> | Have you seen warnings in the TopN case?
>>>>
>>>> I haven't got an isolated TopN test case but it has a similar
>>>> pattern of keeping references to the source iterator but
>>>> not closing via them.
>>>>
>>>> I can construct an example and see what happens.
>>>>
>>>> Chris
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>>>
>>>>>> Hi Andy
>>>>>>
>>>>>> Yes, the cancel thread doesn't call open(), I understand
>>>>>> that.
>>>>>>
>>>>>
>>>>> is that "close()" not "open()"?
>>>>>
>>>>> The warnings are intermittent because the iterator
>>>>>> in question (the "source" interator of QueryIterSort)
>>>>>> is only left open if the cancel request is noticed while
>>>>>> the sort databag is filling up.
>>>>>>
>>>>>> QueryIterSort never explicitly closes the source
>>>>>> iterator. But if the sort databag pulls all of the
>>>>>> source bindings, the source iterator self-closes.
>>>>>> So there's a window while the bag is filling when
>>>>>> a detected cancellation will leave the source
>>>>>> iterator open.
>>>>>>
>>>>>> A fix is to ensure that the source iterator
>>>>>> is closed when QueryIterSort closes:
>>>>>>
>>>>>> @Override public void closeIterator() {
>>>>>>
>>>>>
>>>>> and also:
>>>>>
>>>>> this.db.close();
>>>>>
>>>>> c.f. requestCancel
>>>>>
>>>>> embeddedIterator.close();
>>>>>> super.closeIterator();
>>>>>> }
>>>>>>
>>>>>> "embeddedIterator" is the source iterator.
>>>>>>
>>>>>
>>>>> SortedDataBag: add close() to cancel:
>>>>>
>>>>> public void cancel() {
>>>>> comparator.cancel();
>>>>> close();
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>> This fix eliminates the presenting problem and
>>>>>> the tests continue to pass /except/ that
>>>>>> CallBackIterator's close() method needs to
>>>>>> be made a no-op rather than failing, since
>>>>>> that open() is now actually called.
>>>>>>
>>>>>
>>>>> close()?
>>>>>
>>>>>
>>>>>> [It looks like QueryIterTopN has the same problem]
>>>>>>
>>>>>
>>>>> It would do no harm to have the same close processing in QueryIterTopN
>>>>> though it does not use data bags because it is an unconditional single pass
>>>>> over the data.
>>>>>
>>>>> Have you seen warnings in the TopN case?
>>>>>
>>>>> Andy
>>>>>
>>>>>
>>>>>
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 20 March 2017 at 15:48, Chris Dollin <chris.dollin@epimorphics.com
>>>>>> >
>>>>>> wrote:
>>>>>>
>>>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> SortedDataBag.cancel does not call close.
>>>>>>>>
>>>>>>>>
>>>>>>> You're quite right, am having another look at the problem.
>>>>>>>
>>>>>>> Chris
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>> --
>>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>>> Affair/
>>>>
>>>> Epimorphics Ltd, http://www.epimorphics.com
>>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>>> BS20 6PT
>>>> Epimorphics Ltd. is a limited company registered in England (number
>>>> 7016688)
>>>>
>>>
>>>
>>>
>>> --
>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>> Affair/
>>>
>>> Epimorphics Ltd, http://www.epimorphics.com
>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>> BS20 6PT
>>> Epimorphics Ltd. is a limited company registered in England (number
>>> 7016688)
>>>
>>
>>
>>
>> --
>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>> Affair/
>>
>> Epimorphics Ltd, http://www.epimorphics.com
>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>> BS20 6PT
>> Epimorphics Ltd. is a limited company registered in England (number
>> 7016688)
>>
>
>
>
> --
> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
> Affair/
>
> Epimorphics Ltd, http://www.epimorphics.com
> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
> 6PT
> Epimorphics Ltd. is a limited company registered in England (number
> 7016688)
>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
Whoops, send with no content -- fumble-fingered.
On 4 April 2017 at 12:43, Chris Dollin <ch...@epimorphics.com> wrote:
>
>
> On 4 April 2017 at 09:37, Chris Dollin <ch...@epimorphics.com>
> wrote:
>
>> I ran a simplified version of the test code, with no
>> sort but with a limit, and could not produce warnings.
>>
>
Well of course not, it needs sort + small limit to involve TopN.
Chris
> So I'd not do anything to change TopN unless/until we
>> have more warnings to work with.
>>
>> Have we stabilised enough that I turn this into a JIRA/PR with
>> a unit test?
>>
>> Chris
>>
>>
>> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com>
>> wrote:
>>
>>> Argh, yes, all occurences of "open()" are supposed
>>> to be "close()".
>>>
>>> | Have you seen warnings in the TopN case?
>>>
>>> I haven't got an isolated TopN test case but it has a similar
>>> pattern of keeping references to the source iterator but
>>> not closing via them.
>>>
>>> I can construct an example and see what happens.
>>>
>>> Chris
>>>
>>>
>>>
>>>
>>>
>>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>>
>>>>
>>>>
>>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>>
>>>>> Hi Andy
>>>>>
>>>>> Yes, the cancel thread doesn't call open(), I understand
>>>>> that.
>>>>>
>>>>
>>>> is that "close()" not "open()"?
>>>>
>>>> The warnings are intermittent because the iterator
>>>>> in question (the "source" interator of QueryIterSort)
>>>>> is only left open if the cancel request is noticed while
>>>>> the sort databag is filling up.
>>>>>
>>>>> QueryIterSort never explicitly closes the source
>>>>> iterator. But if the sort databag pulls all of the
>>>>> source bindings, the source iterator self-closes.
>>>>> So there's a window while the bag is filling when
>>>>> a detected cancellation will leave the source
>>>>> iterator open.
>>>>>
>>>>> A fix is to ensure that the source iterator
>>>>> is closed when QueryIterSort closes:
>>>>>
>>>>> @Override public void closeIterator() {
>>>>>
>>>>
>>>> and also:
>>>>
>>>> this.db.close();
>>>>
>>>> c.f. requestCancel
>>>>
>>>> embeddedIterator.close();
>>>>> super.closeIterator();
>>>>> }
>>>>>
>>>>> "embeddedIterator" is the source iterator.
>>>>>
>>>>
>>>> SortedDataBag: add close() to cancel:
>>>>
>>>> public void cancel() {
>>>> comparator.cancel();
>>>> close();
>>>> }
>>>>
>>>>
>>>>
>>>>> This fix eliminates the presenting problem and
>>>>> the tests continue to pass /except/ that
>>>>> CallBackIterator's close() method needs to
>>>>> be made a no-op rather than failing, since
>>>>> that open() is now actually called.
>>>>>
>>>>
>>>> close()?
>>>>
>>>>
>>>>> [It looks like QueryIterTopN has the same problem]
>>>>>
>>>>
>>>> It would do no harm to have the same close processing in QueryIterTopN
>>>> though it does not use data bags because it is an unconditional single pass
>>>> over the data.
>>>>
>>>> Have you seen warnings in the TopN case?
>>>>
>>>> Andy
>>>>
>>>>
>>>>
>>>>> Chris
>>>>>
>>>>>
>>>>>
>>>>> On 20 March 2017 at 15:48, Chris Dollin <ch...@epimorphics.com>
>>>>> wrote:
>>>>>
>>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>>
>>>>>>
>>>>>>> SortedDataBag.cancel does not call close.
>>>>>>>
>>>>>>>
>>>>>> You're quite right, am having another look at the problem.
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>>> --
>>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>>> Affair/
>>>
>>> Epimorphics Ltd, http://www.epimorphics.com
>>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>>> BS20 6PT
>>> Epimorphics Ltd. is a limited company registered in England (number
>>> 7016688)
>>>
>>
>>
>>
>> --
>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>> Affair/
>>
>> Epimorphics Ltd, http://www.epimorphics.com
>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>> BS20 6PT
>> Epimorphics Ltd. is a limited company registered in England (number
>> 7016688)
>>
>
>
>
> --
> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
> Affair/
>
> Epimorphics Ltd, http://www.epimorphics.com
> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
> 6PT
> Epimorphics Ltd. is a limited company registered in England (number
> 7016688)
>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
On 4 April 2017 at 09:37, Chris Dollin <ch...@epimorphics.com> wrote:
> I ran a simplified version of the test code, with no
> sort but with a limit, and could not produce warnings.
> So I'd not do anything to change TopN unless/until we
> have more warnings to work with.
>
> Have we stabilised enough that I turn this into a JIRA/PR with
> a unit test?
>
> Chris
>
>
> On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com>
> wrote:
>
>> Argh, yes, all occurences of "open()" are supposed
>> to be "close()".
>>
>> | Have you seen warnings in the TopN case?
>>
>> I haven't got an isolated TopN test case but it has a similar
>> pattern of keeping references to the source iterator but
>> not closing via them.
>>
>> I can construct an example and see what happens.
>>
>> Chris
>>
>>
>>
>>
>>
>> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>>
>>>
>>>
>>> On 31/03/17 16:40, Chris Dollin wrote:
>>>
>>>> Hi Andy
>>>>
>>>> Yes, the cancel thread doesn't call open(), I understand
>>>> that.
>>>>
>>>
>>> is that "close()" not "open()"?
>>>
>>> The warnings are intermittent because the iterator
>>>> in question (the "source" interator of QueryIterSort)
>>>> is only left open if the cancel request is noticed while
>>>> the sort databag is filling up.
>>>>
>>>> QueryIterSort never explicitly closes the source
>>>> iterator. But if the sort databag pulls all of the
>>>> source bindings, the source iterator self-closes.
>>>> So there's a window while the bag is filling when
>>>> a detected cancellation will leave the source
>>>> iterator open.
>>>>
>>>> A fix is to ensure that the source iterator
>>>> is closed when QueryIterSort closes:
>>>>
>>>> @Override public void closeIterator() {
>>>>
>>>
>>> and also:
>>>
>>> this.db.close();
>>>
>>> c.f. requestCancel
>>>
>>> embeddedIterator.close();
>>>> super.closeIterator();
>>>> }
>>>>
>>>> "embeddedIterator" is the source iterator.
>>>>
>>>
>>> SortedDataBag: add close() to cancel:
>>>
>>> public void cancel() {
>>> comparator.cancel();
>>> close();
>>> }
>>>
>>>
>>>
>>>> This fix eliminates the presenting problem and
>>>> the tests continue to pass /except/ that
>>>> CallBackIterator's close() method needs to
>>>> be made a no-op rather than failing, since
>>>> that open() is now actually called.
>>>>
>>>
>>> close()?
>>>
>>>
>>>> [It looks like QueryIterTopN has the same problem]
>>>>
>>>
>>> It would do no harm to have the same close processing in QueryIterTopN
>>> though it does not use data bags because it is an unconditional single pass
>>> over the data.
>>>
>>> Have you seen warnings in the TopN case?
>>>
>>> Andy
>>>
>>>
>>>
>>>> Chris
>>>>
>>>>
>>>>
>>>> On 20 March 2017 at 15:48, Chris Dollin <ch...@epimorphics.com>
>>>> wrote:
>>>>
>>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>>
>>>>>
>>>>>> SortedDataBag.cancel does not call close.
>>>>>>
>>>>>>
>>>>> You're quite right, am having another look at the problem.
>>>>>
>>>>> Chris
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>
>>
>> --
>> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
>> Affair/
>>
>> Epimorphics Ltd, http://www.epimorphics.com
>> Registered address: Court Lodge, 105 High Street, Portishead, Bristol
>> BS20 6PT
>> Epimorphics Ltd. is a limited company registered in England (number
>> 7016688)
>>
>
>
>
> --
> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
> Affair/
>
> Epimorphics Ltd, http://www.epimorphics.com
> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
> 6PT
> Epimorphics Ltd. is a limited company registered in England (number
> 7016688)
>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
I ran a simplified version of the test code, with no
sort but with a limit, and could not produce warnings.
So I'd not do anything to change TopN unless/until we
have more warnings to work with.
Have we stabilised enough that I turn this into a JIRA/PR with
a unit test?
Chris
On 3 April 2017 at 15:34, Chris Dollin <ch...@epimorphics.com> wrote:
> Argh, yes, all occurences of "open()" are supposed
> to be "close()".
>
> | Have you seen warnings in the TopN case?
>
> I haven't got an isolated TopN test case but it has a similar
> pattern of keeping references to the source iterator but
> not closing via them.
>
> I can construct an example and see what happens.
>
> Chris
>
>
>
>
>
> On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>
>>
>>
>> On 31/03/17 16:40, Chris Dollin wrote:
>>
>>> Hi Andy
>>>
>>> Yes, the cancel thread doesn't call open(), I understand
>>> that.
>>>
>>
>> is that "close()" not "open()"?
>>
>> The warnings are intermittent because the iterator
>>> in question (the "source" interator of QueryIterSort)
>>> is only left open if the cancel request is noticed while
>>> the sort databag is filling up.
>>>
>>> QueryIterSort never explicitly closes the source
>>> iterator. But if the sort databag pulls all of the
>>> source bindings, the source iterator self-closes.
>>> So there's a window while the bag is filling when
>>> a detected cancellation will leave the source
>>> iterator open.
>>>
>>> A fix is to ensure that the source iterator
>>> is closed when QueryIterSort closes:
>>>
>>> @Override public void closeIterator() {
>>>
>>
>> and also:
>>
>> this.db.close();
>>
>> c.f. requestCancel
>>
>> embeddedIterator.close();
>>> super.closeIterator();
>>> }
>>>
>>> "embeddedIterator" is the source iterator.
>>>
>>
>> SortedDataBag: add close() to cancel:
>>
>> public void cancel() {
>> comparator.cancel();
>> close();
>> }
>>
>>
>>
>>> This fix eliminates the presenting problem and
>>> the tests continue to pass /except/ that
>>> CallBackIterator's close() method needs to
>>> be made a no-op rather than failing, since
>>> that open() is now actually called.
>>>
>>
>> close()?
>>
>>
>>> [It looks like QueryIterTopN has the same problem]
>>>
>>
>> It would do no harm to have the same close processing in QueryIterTopN
>> though it does not use data bags because it is an unconditional single pass
>> over the data.
>>
>> Have you seen warnings in the TopN case?
>>
>> Andy
>>
>>
>>
>>> Chris
>>>
>>>
>>>
>>> On 20 March 2017 at 15:48, Chris Dollin <ch...@epimorphics.com>
>>> wrote:
>>>
>>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>>
>>>>
>>>>> SortedDataBag.cancel does not call close.
>>>>>
>>>>>
>>>> You're quite right, am having another look at the problem.
>>>>
>>>> Chris
>>>>
>>>>
>>>
>>>
>>>
>
>
> --
> "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
> Affair/
>
> Epimorphics Ltd, http://www.epimorphics.com
> Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
> 6PT
> Epimorphics Ltd. is a limited company registered in England (number
> 7016688)
>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)
Re: Bug in SortedDataBag?
Posted by Chris Dollin <ch...@epimorphics.com>.
Argh, yes, all occurences of "open()" are supposed
to be "close()".
| Have you seen warnings in the TopN case?
I haven't got an isolated TopN test case but it has a similar
pattern of keeping references to the source iterator but
not closing via them.
I can construct an example and see what happens.
Chris
On 3 April 2017 at 11:15, Andy Seaborne <an...@apache.org> wrote:
>
>
> On 31/03/17 16:40, Chris Dollin wrote:
>
>> Hi Andy
>>
>> Yes, the cancel thread doesn't call open(), I understand
>> that.
>>
>
> is that "close()" not "open()"?
>
> The warnings are intermittent because the iterator
>> in question (the "source" interator of QueryIterSort)
>> is only left open if the cancel request is noticed while
>> the sort databag is filling up.
>>
>> QueryIterSort never explicitly closes the source
>> iterator. But if the sort databag pulls all of the
>> source bindings, the source iterator self-closes.
>> So there's a window while the bag is filling when
>> a detected cancellation will leave the source
>> iterator open.
>>
>> A fix is to ensure that the source iterator
>> is closed when QueryIterSort closes:
>>
>> @Override public void closeIterator() {
>>
>
> and also:
>
> this.db.close();
>
> c.f. requestCancel
>
> embeddedIterator.close();
>> super.closeIterator();
>> }
>>
>> "embeddedIterator" is the source iterator.
>>
>
> SortedDataBag: add close() to cancel:
>
> public void cancel() {
> comparator.cancel();
> close();
> }
>
>
>
>> This fix eliminates the presenting problem and
>> the tests continue to pass /except/ that
>> CallBackIterator's close() method needs to
>> be made a no-op rather than failing, since
>> that open() is now actually called.
>>
>
> close()?
>
>
>> [It looks like QueryIterTopN has the same problem]
>>
>
> It would do no harm to have the same close processing in QueryIterTopN
> though it does not use data bags because it is an unconditional single pass
> over the data.
>
> Have you seen warnings in the TopN case?
>
> Andy
>
>
>
>> Chris
>>
>>
>>
>> On 20 March 2017 at 15:48, Chris Dollin <ch...@epimorphics.com>
>> wrote:
>>
>> On 17 March 2017 at 17:08, Andy Seaborne <an...@apache.org> wrote:
>>>
>>>
>>>> SortedDataBag.cancel does not call close.
>>>>
>>>>
>>> You're quite right, am having another look at the problem.
>>>
>>> Chris
>>>
>>>
>>
>>
>>
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number 7016688)