cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Kulp <dk...@apache.org>
Subject Re: CXF continuation enhancement
Date Wed, 08 Sep 2010 14:23:09 GMT



FYI for everyone else:  log of a quick IRC chat I had with Willem for part of 
my review:

[09:59] <dkulp> Could we avoid using a Message property?  The call to 
getContextual property over and over again is a bit expensive.
[09:59] <willem> cool, I'm writing a mail for it.
[09:59] <dkulp> I was actually thinking just having a suspend() method on 
PhaseInterceptorChain that the continuation would call.
[09:59] <dkulp> sets a boolean right on the chain.
[10:00] <willem> en, that could be more easy to do.
[10:00] <willem> but I need to get the PhaseInterceptorChain instance from the 
message.
[10:00] <dkulp> Yep.  The message holds it.
[10:01] <dkulp> message.getInterceptorChain()


Also, I have one more comment.  In the patch, you have:

+        public void onTimeout(AsyncEvent event) throws IOException {            
             redispatch();
+            isPending = false;
         }
....

         public boolean suspend(long timeout) {
+            if (isPending) {
+                return false;
+            }

which would completely prevent suspend from being able to be called again 
during a redispatch that occurred for a timeout event.

I think the isPending=false needs to move to before the redispatch() call.  
Not 100% positive though.


On Wednesday 08 September 2010 10:02:50 am Willem Jiang wrote:
> Hi Dan,
> 
> Daniel Kulp wrote:
> > On Thursday 02 September 2010 9:32:46 am Willem Jiang wrote:
> >> Hi
> >> 
> >> I just did some work to let the camel-cxf leverage the CXF continuation
> >> framework which you did. I just found current CXF continuation which
> >> suspend is implemented by throw the runtime exception. This
> >> implementation has a shortcoming which cannot call the other framework's
> >> async API after continuation suspend is called as Jetty7 does.
> >> 
> >> This will introduce a situation that continuation resume will be called
> >> before the continuation suspend is called if the famework's async API is
> >> finally using the thread to do the job.
> >> 
> >> So I'm thinking of some enhancement could be possible to let CXF
> >> continuation leverage Jetty 7 new continuation API.
> >> 
> >> Any idea?
> 
> I created a JIRA[1] and submit a patch for it. Please review it :)
> 
> > While upgrading things to Jetty 7, this is something I was thinking about
> > a bit more as well.    I actually wanted to add a method like:
> > 
> > public void resume(Object r);
> > 
> > method onto the continuation.   The endpoint (or background thread or
> > whatever) could call that directly (the Object being the thing that would
> > normally be returned from the method).  The runtime could immediately
> > just grab the associated chain and pick up where it left off and not do
> > the restart thing.   The new Servlet 3 API's and the new things in Jetty
> > 7 seem to allow that.   It could potentially make the continuations
> > stuff a lot easier to work with.
> 
> Current we just make the interceptor chain resume from where we pause
> it. I don't know that you want.
> I think the blow code is enough.
>   continuation.setObject(obj);
>   continuation.resume()
> 

I think my point was that why have it resume where it left off at all.   Why 
could we not just do something like:


Foo doSomething(...) {
....
    if (continuationProvider != null) {
         final Coninuation cont = coninuationProvider.getContinuation();
         cont.suspend();
         executor.execute(new Runnable() {
           public void run() {  
               cont.resume(doTheRealThing(....));
           }
         });
         return null;
    } 
    // no continuation support, must do syncronous
	 return doTheRealThing(...);
}
Foo doTheRealThing(...) {
    return new Foo();
}

With a setup like that, there isn't a need for doSomething(...) to be called 
again at all.   The runtime would be able to handle it all.  For some 
transports, we could use the thread that called resume(obj) to restart the 
chain and such and not bother with another transport provided thread and do 
the tread context switches and such.

Dan


> > I actually would like to change everything to not bother with the
> > exception either.   The endpoint would call "suspend" and if it returns 
> > true (meaning the request was suspended), just return null.  If false,
> > it knows it needs to do the work synchronously.   It's much closer to
> > how the Servlet 3 things work.   Unfortunately, that would completely
> > change the semantics of the API and would require some good docs for the
> > migration guide.
> 
> I don't think this is a good idea, if we can't support to the
> continuation API from the transport level, we simple don't let the user
> can get the ContinuationProvider from the message context.
> 
> It could be much easier for user to use :)
> 
> [1]https://issues.apache.org/jira/browse/CXF-2982
> 
> Willem

-- 
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog

Mime
View raw message