SoftInstigate / restheart

Rapid API Development with MongoDB
https://restheart.org
GNU Affero General Public License v3.0
807 stars 171 forks source link

Exchange already complete exceptions #299

Closed christiangroth closed 5 years ago

christiangroth commented 6 years ago

We encountered quite a few "exchange already complete" exceptions while pushing data to restheart. The stacktraces come in two flavors, so to say. Sometimes it's the MetricsInstrumentationHandler that crashes and sometimes the RequestLoggerHandler.

First kind of exception:

12:31:01.171 [XNIO-1 task-14] ERROR org.restheart.handlers.ErrorHandler - Error handling the request java.lang.IllegalStateException: UT000139: Exchange already complete at io.undertow.server.HttpServerExchange.addExchangeCompleteListener(HttpServerExchange.java:941) at org.restheart.handlers.MetricsInstrumentationHandler.handleRequest(MetricsInstrumentationHandler.java:50) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:655) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:660) at io.undertow.server.handlers.PathHandler.handleRequest(PathHandler.java:94) at io.undertow.server.handlers.HttpContinueAcceptingHandler.handleRequest(HttpContinueAcceptingHandler.java:83) at org.restheart.handlers.ErrorHandler.handleRequest(ErrorHandler.java:70) at io.undertow.server.handlers.encoding.EncodingHandler.handleRequest(EncodingHandler.java:66) at org.restheart.handlers.GzipEncodingHandler.handleRequest(GzipEncodingHandler.java:75) at io.undertow.server.Connectors.executeRootHandler(Connectors.java:336) at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:748)

Second kind of exception:

07:10:49.565 [XNIO-1 task-14] ERROR org.restheart.handlers.ErrorHandler - Error handling the request java.lang.IllegalStateException: UT000139: Exchange already complete at io.undertow.server.HttpServerExchange.addExchangeCompleteListener(HttpServerExchange.java:941) at org.restheart.handlers.RequestLoggerHandler.addExchangeCompleteListener(RequestLoggerHandler.java:168) at org.restheart.handlers.RequestLoggerHandler.dumpExchange(RequestLoggerHandler.java:164) at org.restheart.handlers.RequestLoggerHandler.handleRequest(RequestLoggerHandler.java:80) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.MetricsInstrumentationHandler.handleRequest(MetricsInstrumentationHandler.java:65) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:665) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:670) at io.undertow.server.handlers.PathHandler.handleRequest(PathHandler.java:94) at io.undertow.server.handlers.HttpContinueAcceptingHandler.handleRequest(HttpContinueAcceptingHandler.java:83) at org.restheart.handlers.ErrorHandler.handleRequest(ErrorHandler.java:70) at io.undertow.server.handlers.encoding.EncodingHandler.handleRequest(EncodingHandler.java:66) at org.restheart.handlers.GzipEncodingHandler.handleRequest(GzipEncodingHandler.java:75) at io.undertow.server.Connectors.executeRootHandler(Connectors.java:360) at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:748)

What we have done to further analyze these errors is patching the MetricsInstrumentationHandler based on current master branch to get some more logging in this cases. As this patch should not be needed when #298 is done, I won't provide a PR here, but just paste the diff.

logging-patch.txt

As you can see, the only goal was to provide logging in this cases so we can analyze which requests run into this errors. What we've found out is, that

We've never seen this happen on any other collection than assets.files or with another request method than GET. This of course is just what we've seen and does not mean this can't ever happen in other cases.

Further we noticed this errors more often when pushing data in parallel to restheart (8 threads). But we also have seen this error when pushing data single threaded, but this is a really really really rare case. So paralelism increases the chance to get this error but may not be the root cause.

As we have seen so far the client does not run into any errors and all request are responded as expected, mostly with HTTP 200 OK. So this does not crash anything or produce errors on client side. But the effect might be, that either logging is missing or metrics for these requests are not collected.

Expected Behavior

No errors. ;)

Current Behavior

See above.

Context

Can't provide any more details here, this seems to be quite a general thing.

Environment

Happens using restheart 3.3.1, 3.3.9, 3.4.0 and current master 3.5.0-SNAPSHOT.

Steps to Reproduce

Unfortunately there is no 'safe way' to reproduce this error. Just push assets using multiple threads. We do not have a separate test case or script to reproduce this issue, it just happens in day to day life using our application.

ujibang commented 6 years ago

Hi,

I just pushed a simple commit that avoids exchange.addExchangeCompleteListener() call if exchange is complete in MetricsInstrumentationHandler

This should avoid that exception log and allow to further debug this issue.

Please try your use case with latest 3.5.0-SNAPSHOT.

Few question:

christiangroth commented 6 years ago

Hi @ujibang ,

I will test this with the latest version, but as far as I can see, this will only prevent the exception and not fix the root case, right? Also this will only fix the case in MetricsInstrumentationHandler and not in RequestLoggerHandler (see second kind of exception in initial issue post).

As far as I understand this issue, there might be something going wrong and we only see the consequence here resulting in the exception.

Regardig your questions

grossmueller-espirit commented 6 years ago

It is exactly like @christiangroth said:

Also this will only fix the case in MetricsInstrumentationHandler and not in RequestLoggerHandler (see second kind of exception in initial issue post).

The following error still occurs with RH 3.4.2:

java.lang.IllegalStateException: UT000139: Exchange already complete at io.undertow.server.HttpServerExchange.addExchangeCompleteListener(HttpServerExchange.java:941) at org.restheart.handlers.RequestLoggerHandler.addExchangeCompleteListener(RequestLoggerHandler.java:168) at org.restheart.handlers.RequestLoggerHandler.dumpExchange(RequestLoggerHandler.java:164) at org.restheart.handlers.RequestLoggerHandler.handleRequest(RequestLoggerHandler.java:80) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.TracingInstrumentationHandler.handleRequest(TracingInstrumentationHandler.java:39) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.MetricsInstrumentationHandler.handleRequest(MetricsInstrumentationHandler.java:59) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:665) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:670) at io.undertow.server.handlers.PathHandler.handleRequest(PathHandler.java:94) at io.undertow.server.handlers.HttpContinueAcceptingHandler.handleRequest(HttpContinueAcceptingHandler.java:83) at org.restheart.handlers.ErrorHandler.handleRequest(ErrorHandler.java:70) at io.undertow.server.handlers.encoding.EncodingHandler.handleRequest(EncodingHandler.java:72) at org.restheart.handlers.GzipEncodingHandler.handleRequest(GzipEncodingHandler.java:75) at io.undertow.server.Connectors.executeRootHandler(Connectors.java:360) at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:748)

Additionally, we see this occasionally: java.lang.IllegalStateException: UT000005: getRequestChannel() has already been called at io.undertow.server.HttpServerExchange.setMaxEntitySize(HttpServerExchange.java:1810) at io.undertow.server.handlers.form.MultiPartParserDefinition.create(MultiPartParserDefinition.java:96) at io.undertow.server.handlers.form.FormParserFactory.createParser(FormParserFactory.java:59) at org.restheart.handlers.injectors.BodyInjectorHandler.handleRequest(BodyInjectorHandler.java:296) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.OptionsHandler.handleRequest(OptionsHandler.java:58) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.security.handlers.CORSHandler.handleRequest(CORSHandler.java:88) at org.restheart.handlers.RequestLoggerHandler.handleRequest(RequestLoggerHandler.java:84) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.TracingInstrumentationHandler.handleRequest(TracingInstrumentationHandler.java:39) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.MetricsInstrumentationHandler.handleRequest(MetricsInstrumentationHandler.java:59) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:665) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:670) at io.undertow.server.handlers.PathHandler.handleRequest(PathHandler.java:94) at io.undertow.server.handlers.HttpContinueAcceptingHandler.handleRequest(HttpContinueAcceptingHandler.java:83) at org.restheart.handlers.ErrorHandler.handleRequest(ErrorHandler.java:70) at io.undertow.server.handlers.encoding.EncodingHandler.handleRequest(EncodingHandler.java:72) at org.restheart.handlers.GzipEncodingHandler.handleRequest(GzipEncodingHandler.java:75) at io.undertow.server.Connectors.executeRootHandler(Connectors.java:360) at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:748)

So whatever the underlying problem is, it still persists :-(

ujibang commented 6 years ago

Yep, I knew that my commit was not going to fix it. It is intended at allowing further debug.

Ps I'm currently out of office. Will look at it asap.

grossmueller-espirit commented 6 years ago

Hello again,

after more testing I was able to reproduce the exception in the MetricsInstrumentationHandler again :-( So the issue still persists...

java.lang.IllegalStateException: UT000139: Exchange already complete at io.undertow.server.HttpServerExchange.addExchangeCompleteListener(HttpServerExchange.java:941) at org.restheart.handlers.MetricsInstrumentationHandler.handleRequest(MetricsInstrumentationHandler.java:51) at org.restheart.handlers.PipedHttpHandler.next(PipedHttpHandler.java:121) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:665) at org.restheart.handlers.injectors.RequestContextInjectorHandler.handleRequest(RequestContextInjectorHandler.java:670) at io.undertow.server.handlers.PathHandler.handleRequest(PathHandler.java:94) at io.undertow.server.handlers.HttpContinueAcceptingHandler.handleRequest(HttpContinueAcceptingHandler.java:83) at org.restheart.handlers.ErrorHandler.handleRequest(ErrorHandler.java:70) at io.undertow.server.handlers.encoding.EncodingHandler.handleRequest(EncodingHandler.java:66) at org.restheart.handlers.GzipEncodingHandler.handleRequest(GzipEncodingHandler.java:75) at io.undertow.server.Connectors.executeRootHandler(Connectors.java:360) at io.undertow.server.HttpServerExchange$1.run(HttpServerExchange.java:830) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:748) RH_Exchange_complete.log

mkjsix commented 6 years ago

@ujibang I'd like to put this fix into 3.5.0 which I want to release within a couple of weeks, if possibile.

grossmueller-espirit commented 6 years ago

@mkjsix I just wanted to point out that ujibang's change was not supposed to "fix" it. The exception can still be provoked in the MetricsInstrumentationHandler. If it does not occur there, then it takes place in other handlers down the pipeline.

ujibang commented 5 years ago

just pushed commit 894715679e53ea74a30652ebf113cb854772eed3 that should fix this.

It needs to be confirmed since I cannot reproduce it.

@lenalebt can you give it a look as well?