Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.6.6] akka-http exceptions on every user request #7822

Closed
fommil opened this issue Sep 13, 2017 · 14 comments
Closed

[2.6.6] akka-http exceptions on every user request #7822

fommil opened this issue Sep 13, 2017 · 14 comments

Comments

@fommil
Copy link

fommil commented Sep 13, 2017

I'm seeing many many exceptions with every user request when using the akka-http backend

2017-09-13-110948_1918x1078_scrot

YourKit really doesn't make it easy to export these things, but here's the best I could get

akka-http-exceptions.zip

@marcospereira
Copy link
Member

Hi @fommil, anything special about the requests (some specific non-standard header, for example)?

@fommil
Copy link
Author

fommil commented Sep 13, 2017

just GET requests, with reasonably large lists of parameters (maybe 10 to 20)... btw @gmethvin has also seen this and prompted me to take a closer look.

@jrudolph
Copy link
Contributor

@fommil Parsing in akka-http uses exceptions for flow control, so these exceptions are expected. They don't collect stack traces and usually shouldn't be in the fast path, so there should be no performance hit. It might make sense to get rid of the RuleNotFoundException, though, since that might trigger for every unknown header.

Do you observe any wrong behavior corresponding to those exceptions?

@jrudolph
Copy link
Contributor

jrudolph commented Sep 14, 2017

Looking at the parseMethod exception, I think we could optimize that as well. I wonder, though, what kind of requests these are? Are there HTTP pipelined requests? Any chance you could share some of those requests?

UPDATE: I see, nothing strange going on, it's basically a weird way of checking that there is no further request on the line. We can add a check in startNewMessage to not try parsing at all if there's no further data in the buffer instead of relying on the exception in that case.

@fommil
Copy link
Author

fommil commented Sep 14, 2017

ok cool, good to know. I am seeing stack traces in yourkit though ... is that being added by the instrumentation? Since we run with yourkit/newrelic instrumentation in prod it may mean that we still get hit by the stack traces.

Also... exceptions for control flow... facepalm

jrudolph added a commit to jrudolph/akka-http that referenced this issue Sep 14, 2017
… result ADT

Before every attempt to parse an unknown (i.e. not modeled) header would
end up in throwing and catching an exception. This does not really make
sense as parboiled already provides for delivering these signals without
an exception.

Refs playframework/playframework#7822
jrudolph added a commit to jrudolph/akka-http that referenced this issue Sep 14, 2017
… result ADT

Before every attempt to parse an unknown (i.e. not modeled) header would
end up in throwing and catching an exception. This does not really make
sense as parboiled already provides for delivering these signals without
an exception.

Refs playframework/playframework#7822
@jrudolph
Copy link
Contributor

Also... exceptions for control flow...

It's one of the best ways of writing well-performing suspendable parsers in a straightforward way without having to employ code generation. A comparable functional variant would need the ability to return a continuation at each single point where a single char is read (= Iteratee like structure). This is at least an order of magnitude slower or a manually created state machine which is so much more tedious to read and maintain.

@jrudolph
Copy link
Contributor

Anyway thanks for the traces, I think they will help improving a few things ;)

jrudolph added a commit to jrudolph/akka-http that referenced this issue Sep 14, 2017
… result ADT

Before every attempt to parse an unknown (i.e. not modeled) header would
end up in throwing and catching an exception. This does not really make
sense as parboiled already provides for delivering these signals without
an exception.

Refs playframework/playframework#7822
@jrudolph
Copy link
Contributor

One of those traces seems to point to an issue with parsing X-Forwarded-For headers. You should probably see warnings in the log for those that couldn't be parsed. Would be interesting to see what's going on there. So, if you could post the warning or an example of such a header that would be useful, @fommil.

@fommil
Copy link
Author

fommil commented Sep 14, 2017

interesting... the headers that we're sending out from the server, or the ones we're receiving?

@fommil
Copy link
Author

fommil commented Sep 15, 2017

@jrudolph it turned out to be simple to find this. we're using "X-Forwarded-For" -> "forwarded" in gatling.

Regarding exceptions for control flow... did you know that in Emacs Lisp (and common lisp) the throw and catch are not for error handling but for non-local exits 😀 https://www.gnu.org/software/emacs/manual/html_node/elisp/Catch-and-Throw.html#Catch-and-Throw

It's amazing how I have such different standards when I'm doing scala (a mere mortal's language) vs lisp (the language of the gods)

I can probably setup YourKit to ignore the exceptions that don't have stacktraces. I'm still not sure why I see stacktraces.

jrudolph added a commit to jrudolph/akka-http that referenced this issue Sep 18, 2017
… result ADT

Before every attempt to parse an unknown (i.e. not modeled) header would
end up in throwing and catching an exception. This does not really make
sense as parboiled already provides for delivering these signals without
an exception.

Refs playframework/playframework#7822
@fommil fommil changed the title [2.6.3] akka-http exceptions on every user request [2.6.6] akka-http exceptions on every user request Oct 10, 2017
@fommil
Copy link
Author

fommil commented Oct 10, 2017

@jrudolph our header was easy to fix, but I'm still seeing HeaderParser$RuleNotFoundException and NotEnoughDataException on every query with stacktraces.

This is flagged as "needs info"... is there anything else I can provide?

@fommil
Copy link
Author

fommil commented Oct 10, 2017

oh, I see, your changes were in akka-http. I really wish github had a way of saying at the bottom of every PR "this is included since version tag ..." not just github hashes

@schmitch
Copy link
Contributor

@fommil is this fixed for you? - So can this issue be closed?

@fommil
Copy link
Author

fommil commented Mar 14, 2018

I guess this should have been closed originally anyway, because it was a bug in akka-something.

@fommil fommil closed this as completed Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants