On 4 April 2017 at 12:44, Chris Dollin <[email protected]> wrote:
> Whoops, send with no content -- fumble-fingered. > > > On 4 April 2017 at 12:43, Chris Dollin <[email protected]> > wrote: > >> >> >> On 4 April 2017 at 09:37, Chris Dollin <[email protected]> >> 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 <[email protected]> >>> 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 <[email protected]> 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 <[email protected] >>>>>> > >>>>>> wrote: >>>>>> >>>>>> On 17 March 2017 at 17:08, Andy Seaborne <[email protected]> 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)
