[
https://issues.apache.org/jira/browse/CAMEL-11731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350203#comment-16350203
]
ASF GitHub Bot commented on CAMEL-11731:
----------------------------------------
onderson commented on issue #2188: CAMEL-11731 - add asynccallback and sync
camel and servlet async APIs
URL: https://github.com/apache/camel/pull/2188#issuecomment-362565907
in doServiceAsync method, all the exceptions must be handled and in finally
block `asyncCallback.done(false)` must be called so that ,after proper error
message provided in HttpServletResponse ,context should be completed as a
result of it. I think i understand what you mean by the importance of ordering,
maybe in callback handler's implementation,
instead of
> public void done(boolean doneSync) {
> if (!doneSync) {
> context.complete();
> }
> }
it should be
> public void done(boolean doneSync) {
> context.complete();
> }
and in here, context should not be completed before camel's async processing
is completed.
So, i am wondering in such case would it be simpler not having camel's async
processing may not be needed if Camel Servlet is already asynchronous.
So would it make the current behaviour close to correct handling?
or it will be more complicated to syncronize both async behaviours somewhere
else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Servlet Component isn't really async
> ------------------------------------
>
> Key: CAMEL-11731
> URL: https://issues.apache.org/jira/browse/CAMEL-11731
> Project: Camel
> Issue Type: Improvement
> Components: camel-servlet
> Affects Versions: 2.18.4, 2.19.2
> Environment: All
> Reporter: Nick Houghton
> Assignee: Önder Sezgin
> Priority: Major
> Fix For: Future
>
>
> So my assumption reading the servlet component doco is that with 2.18+ and a
> Servlet 3+ container, the component supports async, which it kind of does
> with the async=true init config, and there is even a method in CamelServlet
> called "doServiceAsync" but from what i can tell it doesn't really do it in a
> asynchronous manner, where there are no blocked threads while a request is
> awaiting an async operation (like an AHC call for example).
> Looking at:
> https://github.com/apache/camel/blob/master/components/camel-http-common/src/main/java/org/apache/camel/http/common/CamelServlet.java
> While processing a request, we check if we are in async mode and call
> doServiceAsync..
> {code:java}
> @Override
> protected final void service(HttpServletRequest req, HttpServletResponse
> resp) throws ServletException, IOException {
> if (isAsync()) {
> final AsyncContext context = req.startAsync();
> //run async
> context.start(() -> doServiceAsync(context));
> } else {
> doService(req, resp);
> }
> }
> {code}
> then in doServiceAsync() we call doService().. and then we call
> getProcessor().process(exchange), not process(exchange, asyncCallback) which
> is the proper async method..
> {code:java}
> try {
> if (log.isTraceEnabled()) {
> log.trace("Processing request for exchangeId: {}",
> exchange.getExchangeId());
> }
> // process the exchange
> consumer.getProcessor().process(exchange);
> } catch (Exception e) {
> exchange.setException(e);
> }
> {code}
> So the actual behaviour is an inbound request in async mode that ends up just
> blocking waiting for the request to complete, in a totally sync manner. I
> presume this is not the desired behaviour?
> It seems the fix would be to move the doService() logic to doServiceAsync()
> and have doService() call it and use the AsyncProcessorConverterHelper. Or
> the other alternative would be to update the documentation to explicitly note
> that it is actually not async at all.
> I can probably PR it in, just wanted to get thoughts on the actual intention.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
