On 4 April 2017 at 09:37, Chris Dollin <chris.dol...@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 <chris.dol...@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 <a...@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.dol...@epimorphics.com>
>>>> wrote:
>>>>
>>>> On 17 March 2017 at 17:08, Andy Seaborne <a...@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)
