You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by Chris Dollin <ch...@epimorphics.com> on 2017/04/03 14:24:44 UTC
Re: runtime regex pattern and many many log messages
I agree that for the case where the pattern is
constant (passes isConstant ie is a string literal),
your case 1/) it's best treated as a syntax error --
which is the current behaviour, so no change needed
there.
In 2/, converting to a syntax error means (if I read
the code correctly) that the first bad pattern will
terminate the entire query. Is that compatible with
the SPARQL spec? I looked at the relevant sections
and left unsure.
If the query is terminated immediately then there's
no need to use Log.warnOnce?
Chris
On 29 March 2017 at 22:49, Andy Seaborne <an...@apache.org> wrote:
> There are two cases: I'd prefer to leave the static case as a syntax error
> and improve the dynamic case.
>
> 1/ If its a fixed pattern, it is compiled once.
> An error here should become a "syntax error" to
> E_Regex.init via makeRegexEngine.
>
> This is a common case.
>
> 2/ If a dynamic pattern, a ExprEvalException (it has no stacktrace because
> fillInStackTrace is overridden for efficiency) would be better.
>
> So throwing a ExprEvalException, catching in E_Regex.init and then
> throwing ExprException to become a syntax error as currently.
>
> In addition, in (2), Log.warnOnce would be good.
>
> Andy
>
>
> On 24/03/17 11:55, chris dollin wrote:
>
>> Dear All
>>
>> When tracking down the cause of a disc overflow
>> on our Fuseki servers [0], we found that the log
>> files were full of stack dumps caused by queries
>> containing non-constant [1] illegal regular expression
>> patterns.
>>
>> In a query like
>>
>> SELECT * WHERE {?s ?p ?o FILTER(REGEX(?o, ?o + ")DEF", "")}
>>
>> where the pattern part of the regex is not constant,
>> the regex is compiled (in RegexJava) when the filter is
>> evaluated against the solution bindings. If the pattern
>> is syntactically illegal, a PatternSyntaxException is
>> thrown, caught by RegexJava in makePattern, which then
>> thows an ExprException.
>>
>> The ExprException is caught in QueryIterFilterExpr.accept()
>> and a log entry generated, including the (large) stack
>> trace, and the filter is treated as FALSE so no solution
>> is generated.
>>
>> When there are many -- say millions -- of results to filter
>> this generates millions of unhelpful stack traces,
>> [unhelpful because it confuses the evaluation of the
>> SPARQL query with the execution of the Java ARQ code]
>> and the disc fills up with logfile.
>>
>> This is of course an edge case but we'd like to
>> avoid being cut so badly by the edge. Any other
>> filter expression that can throw EvalException will
>> have similar behaviour.
>>
>> At the very least we'd like to be able to reduce the
>> size of each log message by omitting the stack trace [2].
>> But when there are millions of results to filter,
>> there will still be millions of lines going to the
>> logs.
>>
>> An alternative would be to limit the number of warning log
>> entries generated for any given filter.
>>
>> If the pattern part of a regex filter is treated as part
>> of the query rather than part of the data, even in the
>> case above that we started with, then a syntax error in
>> the pattern could be regarded as an error in the query,
>> and the entire query execution abandoned. Making this
>> spec-compatible might be tricky, though.
>>
>> Thoughts?
>>
>> Chris
>>
>> [0] Fuseki 2.4.1
>>
>> [1] When the pattern is a constant, which it will
>> usually be, it is compiled once before the query
>> runs and generates a single different error.
>>
>> [2] One way to do this would be to have RegexJava.makePattern
>> throw an ExprEvalException rather than an ExprException;
>> we note empirically that an ExprEvalException generates
>> a log entry that doesn't contain a stack trace. Or
>> we could introduce additional specific exception types.
>>
>>
>>
--
"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: runtime regex pattern and many many log messages
Posted by Andy Seaborne <an...@apache.org>.
On 10/04/17 15:32, Chris Dollin wrote:
> On 5 April 2017 at 16:15, Andy Seaborne <an...@apache.org> wrote:
>
>> Changes pushed.
>
>
> I don't see them on the github mirror -- am I
> looking in the wrong place?
Updates were pushed to discussion:
https://github.com/afs/jena/tree/epimorphics_reports
I've pjust made a Pull Request #236 whre it might be easier to
see/discuss the changes.
Andy
>
> Finally writing some tests.
>
> Chris
>
>
>>
>>
>> On 05/04/17 15:32, Andy Seaborne wrote:
>>
>>>
>>>
>>> On 05/04/17 15:10, Chris Dollin wrote:
>>>
>>>> I've eyeballed the commits. Thanks, Andy. Two things:
>>>>
>>>> E_Regex.java line 80 "we could non-eval exception."
>>>> <https://github.com/apache/jena/compare/master...afs:epimorp
>>>> hics_reports#diff-35f629317420ff3f22f8a6c927cbe26c>
>>>>
>>>>
>>>> I didn't understand the comment.
>>>>
>>>
>>> "We could throw a non-eval exception."
>>>
>>> as per the commented code on the line after.
>>>
>>> I found that there was no need to rebuild the exception in local
>>> testing. During the expression build step, it is not inside a FILTER
>>> catching ExprEvalException. And the stack is misleading as it would be
>>> moved.
>>>
>>>
>>>> RegexXerces parallels the code of RegexJava and so
>>>> needs a similar change.
>>>>
>>>
>>> Sure - it isn't used by default.
>>>
>>> RegexXerces is only needed for "x" flag which Java does not support.
>>>
>>>
>>>> I will run code when I have unbroken my Eclipse.
>>>>
>>>> Chris
>>>>
>>>> On 5 April 2017 at 14:23, Chris Dollin <ch...@epimorphics.com>
>>>> wrote:
>>>>
>>>> Thanks Andy I will look at them shortly.
>>>>>
>>>>> I can clean up the unit test we used for the cancellation
>>>>> case, it's fast but it only shows that closing the QueryIterSort
>>>>> iterator closes the source iterator to -- I've been wondering
>>>>> how to do an integration test that shows that the "didn't close"
>>>>> warnings have gone away.
>>>>>
>>>>> I will construct a test for the regex changes.
>>>>>
>>>>> Chris
>>>>>
>>>>> (struggling with Eclipse)
>>>>>
>>>>>
>>>>>
>>>>> On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
>>>>>
>>>>> Please take a look at the two commits on
>>>>>>
>>>>>> https://github.com/afs/jena/tree/epimorphics_reports
>>>>>>
>>>>>> One for regex, one for query cancellation in sorts.
>>>>>>
>>>>>> Do you have a test or two (which aren't very slow)?
>>>>>>
>>>>>> Andy
>>>>>>
>>>>>>
>>>>>> On 04/04/17 14:25, Chris Dollin wrote:
>>>>>>
>>>>>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>>>>>>
>>>>>>>> with warning, not a syntax error.
>>>>>>>>
>>>>>>>>
>>>>>>>> OK, I misunderstood.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Is that compatible with
>>>>>>>>
>>>>>>>> the SPARQL spec? I looked at the relevant sections
>>>>>>>>> and left unsure.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The spec says nothing about bad regexs.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> So we /could/ say that evaluation an expression with a
>>>>>>> syntactically illegal pattern terminated the query without
>>>>>>> violating the spec. But we don't need to since the effect
>>>>>>> we wanted was not to have enormous logs with a line
>>>>>>> for each illegal regex, and we can use Log.warnOnce to
>>>>>>> suppress duplicate messages.
>>>>>>>
>>>>>>> [Of course the expression might evaluate differently
>>>>>>> each time so there could still be lots of distinct
>>>>>>> log lines but that's another edg on what's already an
>>>>>>> edge case...]
>>>>>>>
>>>>>>> I am happy with your 1/ and 2/ approach.
>>>>>>>
>>>>>>> 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: runtime regex pattern and many many log messages
Posted by Chris Dollin <ch...@epimorphics.com>.
On 5 April 2017 at 16:15, Andy Seaborne <an...@apache.org> wrote:
> Changes pushed.
I don't see them on the github mirror -- am I
looking in the wrong place?
Finally writing some tests.
Chris
>
>
> On 05/04/17 15:32, Andy Seaborne wrote:
>
>>
>>
>> On 05/04/17 15:10, Chris Dollin wrote:
>>
>>> I've eyeballed the commits. Thanks, Andy. Two things:
>>>
>>> E_Regex.java line 80 "we could non-eval exception."
>>> <https://github.com/apache/jena/compare/master...afs:epimorp
>>> hics_reports#diff-35f629317420ff3f22f8a6c927cbe26c>
>>>
>>>
>>> I didn't understand the comment.
>>>
>>
>> "We could throw a non-eval exception."
>>
>> as per the commented code on the line after.
>>
>> I found that there was no need to rebuild the exception in local
>> testing. During the expression build step, it is not inside a FILTER
>> catching ExprEvalException. And the stack is misleading as it would be
>> moved.
>>
>>
>>> RegexXerces parallels the code of RegexJava and so
>>> needs a similar change.
>>>
>>
>> Sure - it isn't used by default.
>>
>> RegexXerces is only needed for "x" flag which Java does not support.
>>
>>
>>> I will run code when I have unbroken my Eclipse.
>>>
>>> Chris
>>>
>>> On 5 April 2017 at 14:23, Chris Dollin <ch...@epimorphics.com>
>>> wrote:
>>>
>>> Thanks Andy I will look at them shortly.
>>>>
>>>> I can clean up the unit test we used for the cancellation
>>>> case, it's fast but it only shows that closing the QueryIterSort
>>>> iterator closes the source iterator to -- I've been wondering
>>>> how to do an integration test that shows that the "didn't close"
>>>> warnings have gone away.
>>>>
>>>> I will construct a test for the regex changes.
>>>>
>>>> Chris
>>>>
>>>> (struggling with Eclipse)
>>>>
>>>>
>>>>
>>>> On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
>>>>
>>>> Please take a look at the two commits on
>>>>>
>>>>> https://github.com/afs/jena/tree/epimorphics_reports
>>>>>
>>>>> One for regex, one for query cancellation in sorts.
>>>>>
>>>>> Do you have a test or two (which aren't very slow)?
>>>>>
>>>>> Andy
>>>>>
>>>>>
>>>>> On 04/04/17 14:25, Chris Dollin wrote:
>>>>>
>>>>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>>>>>
>>>>>>
>>>>>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>>>>>
>>>>>>> with warning, not a syntax error.
>>>>>>>
>>>>>>>
>>>>>>> OK, I misunderstood.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Is that compatible with
>>>>>>>
>>>>>>> the SPARQL spec? I looked at the relevant sections
>>>>>>>> and left unsure.
>>>>>>>>
>>>>>>>>
>>>>>>>> The spec says nothing about bad regexs.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> So we /could/ say that evaluation an expression with a
>>>>>> syntactically illegal pattern terminated the query without
>>>>>> violating the spec. But we don't need to since the effect
>>>>>> we wanted was not to have enormous logs with a line
>>>>>> for each illegal regex, and we can use Log.warnOnce to
>>>>>> suppress duplicate messages.
>>>>>>
>>>>>> [Of course the expression might evaluate differently
>>>>>> each time so there could still be lots of distinct
>>>>>> log lines but that's another edg on what's already an
>>>>>> edge case...]
>>>>>>
>>>>>> I am happy with your 1/ and 2/ approach.
>>>>>>
>>>>>> 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: runtime regex pattern and many many log messages
Posted by Andy Seaborne <an...@apache.org>.
Changes pushed.
On 05/04/17 15:32, Andy Seaborne wrote:
>
>
> On 05/04/17 15:10, Chris Dollin wrote:
>> I've eyeballed the commits. Thanks, Andy. Two things:
>>
>> E_Regex.java line 80 "we could non-eval exception."
>> <https://github.com/apache/jena/compare/master...afs:epimorphics_reports#diff-35f629317420ff3f22f8a6c927cbe26c>
>>
>>
>> I didn't understand the comment.
>
> "We could throw a non-eval exception."
>
> as per the commented code on the line after.
>
> I found that there was no need to rebuild the exception in local
> testing. During the expression build step, it is not inside a FILTER
> catching ExprEvalException. And the stack is misleading as it would be
> moved.
>
>>
>> RegexXerces parallels the code of RegexJava and so
>> needs a similar change.
>
> Sure - it isn't used by default.
>
> RegexXerces is only needed for "x" flag which Java does not support.
>
>>
>> I will run code when I have unbroken my Eclipse.
>>
>> Chris
>>
>> On 5 April 2017 at 14:23, Chris Dollin <ch...@epimorphics.com>
>> wrote:
>>
>>> Thanks Andy I will look at them shortly.
>>>
>>> I can clean up the unit test we used for the cancellation
>>> case, it's fast but it only shows that closing the QueryIterSort
>>> iterator closes the source iterator to -- I've been wondering
>>> how to do an integration test that shows that the "didn't close"
>>> warnings have gone away.
>>>
>>> I will construct a test for the regex changes.
>>>
>>> Chris
>>>
>>> (struggling with Eclipse)
>>>
>>>
>>>
>>> On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
>>>
>>>> Please take a look at the two commits on
>>>>
>>>> https://github.com/afs/jena/tree/epimorphics_reports
>>>>
>>>> One for regex, one for query cancellation in sorts.
>>>>
>>>> Do you have a test or two (which aren't very slow)?
>>>>
>>>> Andy
>>>>
>>>>
>>>> On 04/04/17 14:25, Chris Dollin wrote:
>>>>
>>>>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>>>>
>>>>>
>>>>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>>>>> with warning, not a syntax error.
>>>>>>
>>>>>>
>>>>> OK, I misunderstood.
>>>>>
>>>>>
>>>>>
>>>>>> Is that compatible with
>>>>>>
>>>>>>> the SPARQL spec? I looked at the relevant sections
>>>>>>> and left unsure.
>>>>>>>
>>>>>>>
>>>>>> The spec says nothing about bad regexs.
>>>>>>
>>>>>
>>>>>
>>>>> So we /could/ say that evaluation an expression with a
>>>>> syntactically illegal pattern terminated the query without
>>>>> violating the spec. But we don't need to since the effect
>>>>> we wanted was not to have enormous logs with a line
>>>>> for each illegal regex, and we can use Log.warnOnce to
>>>>> suppress duplicate messages.
>>>>>
>>>>> [Of course the expression might evaluate differently
>>>>> each time so there could still be lots of distinct
>>>>> log lines but that's another edg on what's already an
>>>>> edge case...]
>>>>>
>>>>> I am happy with your 1/ and 2/ approach.
>>>>>
>>>>> 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: runtime regex pattern and many many log messages
Posted by Andy Seaborne <an...@apache.org>.
On 05/04/17 15:10, Chris Dollin wrote:
> I've eyeballed the commits. Thanks, Andy. Two things:
>
> E_Regex.java line 80 "we could non-eval exception."
> <https://github.com/apache/jena/compare/master...afs:epimorphics_reports#diff-35f629317420ff3f22f8a6c927cbe26c>
>
> I didn't understand the comment.
"We could throw a non-eval exception."
as per the commented code on the line after.
I found that there was no need to rebuild the exception in local
testing. During the expression build step, it is not inside a FILTER
catching ExprEvalException. And the stack is misleading as it would be
moved.
>
> RegexXerces parallels the code of RegexJava and so
> needs a similar change.
Sure - it isn't used by default.
RegexXerces is only needed for "x" flag which Java does not support.
>
> I will run code when I have unbroken my Eclipse.
>
> Chris
>
> On 5 April 2017 at 14:23, Chris Dollin <ch...@epimorphics.com> wrote:
>
>> Thanks Andy I will look at them shortly.
>>
>> I can clean up the unit test we used for the cancellation
>> case, it's fast but it only shows that closing the QueryIterSort
>> iterator closes the source iterator to -- I've been wondering
>> how to do an integration test that shows that the "didn't close"
>> warnings have gone away.
>>
>> I will construct a test for the regex changes.
>>
>> Chris
>>
>> (struggling with Eclipse)
>>
>>
>>
>> On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
>>
>>> Please take a look at the two commits on
>>>
>>> https://github.com/afs/jena/tree/epimorphics_reports
>>>
>>> One for regex, one for query cancellation in sorts.
>>>
>>> Do you have a test or two (which aren't very slow)?
>>>
>>> Andy
>>>
>>>
>>> On 04/04/17 14:25, Chris Dollin wrote:
>>>
>>>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>>>
>>>>
>>>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>>>> with warning, not a syntax error.
>>>>>
>>>>>
>>>> OK, I misunderstood.
>>>>
>>>>
>>>>
>>>>> Is that compatible with
>>>>>
>>>>>> the SPARQL spec? I looked at the relevant sections
>>>>>> and left unsure.
>>>>>>
>>>>>>
>>>>> The spec says nothing about bad regexs.
>>>>>
>>>>
>>>>
>>>> So we /could/ say that evaluation an expression with a
>>>> syntactically illegal pattern terminated the query without
>>>> violating the spec. But we don't need to since the effect
>>>> we wanted was not to have enormous logs with a line
>>>> for each illegal regex, and we can use Log.warnOnce to
>>>> suppress duplicate messages.
>>>>
>>>> [Of course the expression might evaluate differently
>>>> each time so there could still be lots of distinct
>>>> log lines but that's another edg on what's already an
>>>> edge case...]
>>>>
>>>> I am happy with your 1/ and 2/ approach.
>>>>
>>>> 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: runtime regex pattern and many many log messages
Posted by Chris Dollin <ch...@epimorphics.com>.
I've eyeballed the commits. Thanks, Andy. Two things:
E_Regex.java line 80 "we could non-eval exception."
<https://github.com/apache/jena/compare/master...afs:epimorphics_reports#diff-35f629317420ff3f22f8a6c927cbe26c>
I didn't understand the comment.
RegexXerces parallels the code of RegexJava and so
needs a similar change.
I will run code when I have unbroken my Eclipse.
Chris
On 5 April 2017 at 14:23, Chris Dollin <ch...@epimorphics.com> wrote:
> Thanks Andy I will look at them shortly.
>
> I can clean up the unit test we used for the cancellation
> case, it's fast but it only shows that closing the QueryIterSort
> iterator closes the source iterator to -- I've been wondering
> how to do an integration test that shows that the "didn't close"
> warnings have gone away.
>
> I will construct a test for the regex changes.
>
> Chris
>
> (struggling with Eclipse)
>
>
>
> On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
>
>> Please take a look at the two commits on
>>
>> https://github.com/afs/jena/tree/epimorphics_reports
>>
>> One for regex, one for query cancellation in sorts.
>>
>> Do you have a test or two (which aren't very slow)?
>>
>> Andy
>>
>>
>> On 04/04/17 14:25, Chris Dollin wrote:
>>
>>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>>
>>>
>>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>>> with warning, not a syntax error.
>>>>
>>>>
>>> OK, I misunderstood.
>>>
>>>
>>>
>>>> Is that compatible with
>>>>
>>>>> the SPARQL spec? I looked at the relevant sections
>>>>> and left unsure.
>>>>>
>>>>>
>>>> The spec says nothing about bad regexs.
>>>>
>>>
>>>
>>> So we /could/ say that evaluation an expression with a
>>> syntactically illegal pattern terminated the query without
>>> violating the spec. But we don't need to since the effect
>>> we wanted was not to have enormous logs with a line
>>> for each illegal regex, and we can use Log.warnOnce to
>>> suppress duplicate messages.
>>>
>>> [Of course the expression might evaluate differently
>>> each time so there could still be lots of distinct
>>> log lines but that's another edg on what's already an
>>> edge case...]
>>>
>>> I am happy with your 1/ and 2/ approach.
>>>
>>> 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: runtime regex pattern and many many log messages
Posted by Chris Dollin <ch...@epimorphics.com>.
On 5 April 2017 at 14:23, Chris Dollin <ch...@epimorphics.com> wrote:
> Thanks Andy I will look at them shortly.
>
> I can clean up the unit test we used for the cancellation
> case, it's fast but it only shows that closing the QueryIterSort
> iterator closes the source iterator to -- I've been wondering
> how to do an integration test that shows that the "didn't close"
> warnings have gone away.
>
> I will construct a test for the regex changes.
>
> Chris
>
> (struggling with Eclipse)
>
>
>
> On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
>
>> Please take a look at the two commits on
>>
>> https://github.com/afs/jena/tree/epimorphics_reports
>>
>> One for regex, one for query cancellation in sorts.
>>
>> Do you have a test or two (which aren't very slow)?
>>
>> Andy
>>
>>
>> On 04/04/17 14:25, Chris Dollin wrote:
>>
>>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>>
>>>
>>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>>> with warning, not a syntax error.
>>>>
>>>>
>>> OK, I misunderstood.
>>>
>>>
>>>
>>>> Is that compatible with
>>>>
>>>>> the SPARQL spec? I looked at the relevant sections
>>>>> and left unsure.
>>>>>
>>>>>
>>>> The spec says nothing about bad regexs.
>>>>
>>>
>>>
>>> So we /could/ say that evaluation an expression with a
>>> syntactically illegal pattern terminated the query without
>>> violating the spec. But we don't need to since the effect
>>> we wanted was not to have enormous logs with a line
>>> for each illegal regex, and we can use Log.warnOnce to
>>> suppress duplicate messages.
>>>
>>> [Of course the expression might evaluate differently
>>> each time so there could still be lots of distinct
>>> log lines but that's another edg on what's already an
>>> edge case...]
>>>
>>> I am happy with your 1/ and 2/ approach.
>>>
>>> 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: runtime regex pattern and many many log messages
Posted by Chris Dollin <ch...@epimorphics.com>.
Thanks Andy I will look at them shortly.
I can clean up the unit test we used for the cancellation
case, it's fast but it only shows that closing the QueryIterSort
iterator closes the source iterator to -- I've been wondering
how to do an integration test that shows that the "didn't close"
warnings have gone away.
I will construct a test for the regex changes.
Chris
(struggling with Eclipse)
On 5 April 2017 at 13:15, Andy Seaborne <an...@apache.org> wrote:
> Please take a look at the two commits on
>
> https://github.com/afs/jena/tree/epimorphics_reports
>
> One for regex, one for query cancellation in sorts.
>
> Do you have a test or two (which aren't very slow)?
>
> Andy
>
>
> On 04/04/17 14:25, Chris Dollin wrote:
>
>> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>>
>>
>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>>> with warning, not a syntax error.
>>>
>>>
>> OK, I misunderstood.
>>
>>
>>
>>> Is that compatible with
>>>
>>>> the SPARQL spec? I looked at the relevant sections
>>>> and left unsure.
>>>>
>>>>
>>> The spec says nothing about bad regexs.
>>>
>>
>>
>> So we /could/ say that evaluation an expression with a
>> syntactically illegal pattern terminated the query without
>> violating the spec. But we don't need to since the effect
>> we wanted was not to have enormous logs with a line
>> for each illegal regex, and we can use Log.warnOnce to
>> suppress duplicate messages.
>>
>> [Of course the expression might evaluate differently
>> each time so there could still be lots of distinct
>> log lines but that's another edg on what's already an
>> edge case...]
>>
>> I am happy with your 1/ and 2/ approach.
>>
>> 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: runtime regex pattern and many many log messages
Posted by Andy Seaborne <an...@apache.org>.
Please take a look at the two commits on
https://github.com/afs/jena/tree/epimorphics_reports
One for regex, one for query cancellation in sorts.
Do you have a test or two (which aren't very slow)?
Andy
On 04/04/17 14:25, Chris Dollin wrote:
> On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
>
>
>> What I proposed is to treat bad dynamic regexs as an evaluation error,
>> with warning, not a syntax error.
>>
>
> OK, I misunderstood.
>
>
>>
>> Is that compatible with
>>> the SPARQL spec? I looked at the relevant sections
>>> and left unsure.
>>>
>>
>> The spec says nothing about bad regexs.
>
>
> So we /could/ say that evaluation an expression with a
> syntactically illegal pattern terminated the query without
> violating the spec. But we don't need to since the effect
> we wanted was not to have enormous logs with a line
> for each illegal regex, and we can use Log.warnOnce to
> suppress duplicate messages.
>
> [Of course the expression might evaluate differently
> each time so there could still be lots of distinct
> log lines but that's another edg on what's already an
> edge case...]
>
> I am happy with your 1/ and 2/ approach.
>
> Chris
>
>
>
>
>
>
>
>
Re: runtime regex pattern and many many log messages
Posted by Chris Dollin <ch...@epimorphics.com>.
On 3 April 2017 at 16:30, Andy Seaborne <an...@apache.org> wrote:
> What I proposed is to treat bad dynamic regexs as an evaluation error,
> with warning, not a syntax error.
>
OK, I misunderstood.
>
> Is that compatible with
>> the SPARQL spec? I looked at the relevant sections
>> and left unsure.
>>
>
> The spec says nothing about bad regexs.
So we /could/ say that evaluation an expression with a
syntactically illegal pattern terminated the query without
violating the spec. But we don't need to since the effect
we wanted was not to have enormous logs with a line
for each illegal regex, and we can use Log.warnOnce to
suppress duplicate messages.
[Of course the expression might evaluate differently
each time so there could still be lots of distinct
log lines but that's another edg on what's already an
edge case...]
I am happy with your 1/ and 2/ approach.
Chris
--
"It took a very long time, much longer than the most /Sector
General/
generous estimates." - James
White
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: runtime regex pattern and many many log messages
Posted by Andy Seaborne <an...@apache.org>.
On 03/04/17 15:24, Chris Dollin wrote:
> I agree that for the case where the pattern is
> constant (passes isConstant ie is a string literal),
> your case 1/) it's best treated as a syntax error --
> which is the current behaviour, so no change needed
> there.
>
> In 2/, converting to a syntax error means (if I read
> the code correctly) that the first bad pattern will
> terminate the entire query.
What I proposed is to treat bad dynamic regexs as an evaluation error,
with warning, not a syntax error.
> Is that compatible with
> the SPARQL spec? I looked at the relevant sections
> and left unsure.
The spec says nothing about bad regexs.
Andy
>
> If the query is terminated immediately then there's
> no need to use Log.warnOnce?
>
> Chris
>
>
>
>
>
>
>
> On 29 March 2017 at 22:49, Andy Seaborne <an...@apache.org> wrote:
>
>> There are two cases: I'd prefer to leave the static case as a syntax error
>> and improve the dynamic case.
>>
>> 1/ If its a fixed pattern, it is compiled once.
>> An error here should become a "syntax error" to
>> E_Regex.init via makeRegexEngine.
>>
>> This is a common case.
>>
>> 2/ If a dynamic pattern, a ExprEvalException (it has no stacktrace because
>> fillInStackTrace is overridden for efficiency) would be better.
>>
>> So throwing a ExprEvalException, catching in E_Regex.init and then
>> throwing ExprException to become a syntax error as currently.
>>
>> In addition, in (2), Log.warnOnce would be good.
>>
>> Andy
>>
>>
>> On 24/03/17 11:55, chris dollin wrote:
>>
>>> Dear All
>>>
>>> When tracking down the cause of a disc overflow
>>> on our Fuseki servers [0], we found that the log
>>> files were full of stack dumps caused by queries
>>> containing non-constant [1] illegal regular expression
>>> patterns.
>>>
>>> In a query like
>>>
>>> SELECT * WHERE {?s ?p ?o FILTER(REGEX(?o, ?o + ")DEF", "")}
>>>
>>> where the pattern part of the regex is not constant,
>>> the regex is compiled (in RegexJava) when the filter is
>>> evaluated against the solution bindings. If the pattern
>>> is syntactically illegal, a PatternSyntaxException is
>>> thrown, caught by RegexJava in makePattern, which then
>>> thows an ExprException.
>>>
>>> The ExprException is caught in QueryIterFilterExpr.accept()
>>> and a log entry generated, including the (large) stack
>>> trace, and the filter is treated as FALSE so no solution
>>> is generated.
>>>
>>> When there are many -- say millions -- of results to filter
>>> this generates millions of unhelpful stack traces,
>>> [unhelpful because it confuses the evaluation of the
>>> SPARQL query with the execution of the Java ARQ code]
>>> and the disc fills up with logfile.
>>>
>>> This is of course an edge case but we'd like to
>>> avoid being cut so badly by the edge. Any other
>>> filter expression that can throw EvalException will
>>> have similar behaviour.
>>>
>>> At the very least we'd like to be able to reduce the
>>> size of each log message by omitting the stack trace [2].
>>> But when there are millions of results to filter,
>>> there will still be millions of lines going to the
>>> logs.
>>>
>>> An alternative would be to limit the number of warning log
>>> entries generated for any given filter.
>>>
>>> If the pattern part of a regex filter is treated as part
>>> of the query rather than part of the data, even in the
>>> case above that we started with, then a syntax error in
>>> the pattern could be regarded as an error in the query,
>>> and the entire query execution abandoned. Making this
>>> spec-compatible might be tricky, though.
>>>
>>> Thoughts?
>>>
>>> Chris
>>>
>>> [0] Fuseki 2.4.1
>>>
>>> [1] When the pattern is a constant, which it will
>>> usually be, it is compiled once before the query
>>> runs and generates a single different error.
>>>
>>> [2] One way to do this would be to have RegexJava.makePattern
>>> throw an ExprEvalException rather than an ExprException;
>>> we note empirically that an ExprEvalException generates
>>> a log entry that doesn't contain a stack trace. Or
>>> we could introduce additional specific exception types.
>>>
>>>
>>>
>
>