You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by Daniel Kulp <dk...@apache.org> on 2010/09/02 16:05:49 UTC
Re: CXF continuation enhancement
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?
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.
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.
--
Daniel Kulp
dkulp@apache.org
http://dankulp.com/blog
Re: CXF continuation enhancement
Posted by Willem Jiang <wi...@gmail.com>.
On 9/9/10 10:57 AM, Daniel Kulp wrote:
> On Wednesday 08 September 2010 9:08:36 pm Willem Jiang wrote:
> .......
>> I'm afraid I had to introduce other status into interceptor chain like
>> SUSPEND, it will move the interceptor iterator to the preious one and
>> through the SuspendedInvocationException, and PAUSED will not do these
>> things.
>>
>> I will give it a shoot :)
>
> Cool. Hopefully it works a bit better.
>
> This actually should be a bit cleaner across the board. If the
> ServiceInvokerInterceptor and such needs to see if the chain is suspended, it
> can just call message.getInterceptorChain().isSuspended() (or similar). For
> example, since the Exception in not longer thrown, it may need (or at least
> should) check that to see if it should setup the response message or not.
>
>
I just committed the patch into the check, please review it.
>>>> 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();
>>>
>>> }
>>
>> Understood, in this way the implemenation can support the continuation
>> and no continuation environment at the same time.
>> As we get the control of the interceptor chain, it can be done by
>> hacking the ServiceInvokerInterceptor and put the response back, but it
>> will break the old continuation semantic which can make the user method
>> be called again when the continuation resume is called.
>
> Well, I was kind of assuming that if resume() is called (the old method), the
> old behavior would remain. If resume(object) is called, the new behavior can
> be used. Thus, it really wouldn't break the old semantic, but would allow
> for a new, cleaner, semantic. What do you think?
>
> Dan
Current CXF continuation is not just works for ServiceImpl, it also
helpful if the user uses it in the interceptor. If we introduce the
resume(object) method, this method can't be used in the other
interceptors and it let user misunderstand the continuation's meaning.
So I suggest we don't introduce this method into CXF continuation.
>
>
>
>>
>> Willem
>>
>>> 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
>
Re: CXF continuation enhancement
Posted by Daniel Kulp <dk...@apache.org>.
On Wednesday 08 September 2010 9:08:36 pm Willem Jiang wrote:
.......
> I'm afraid I had to introduce other status into interceptor chain like
> SUSPEND, it will move the interceptor iterator to the preious one and
> through the SuspendedInvocationException, and PAUSED will not do these
> things.
>
> I will give it a shoot :)
Cool. Hopefully it works a bit better.
This actually should be a bit cleaner across the board. If the
ServiceInvokerInterceptor and such needs to see if the chain is suspended, it
can just call message.getInterceptorChain().isSuspended() (or similar). For
example, since the Exception in not longer thrown, it may need (or at least
should) check that to see if it should setup the response message or not.
> >> 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();
> >
> > }
>
> Understood, in this way the implemenation can support the continuation
> and no continuation environment at the same time.
> As we get the control of the interceptor chain, it can be done by
> hacking the ServiceInvokerInterceptor and put the response back, but it
> will break the old continuation semantic which can make the user method
> be called again when the continuation resume is called.
Well, I was kind of assuming that if resume() is called (the old method), the
old behavior would remain. If resume(object) is called, the new behavior can
be used. Thus, it really wouldn't break the old semantic, but would allow
for a new, cleaner, semantic. What do you think?
Dan
>
> Willem
>
> > 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
Re: CXF continuation enhancement
Posted by Willem Jiang <wi...@gmail.com>.
Daniel Kulp wrote:
>
>
> 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()
>
I'm afraid I had to introduce other status into interceptor chain like
SUSPEND, it will move the interceptor iterator to the preious one and
through the SuspendedInvocationException, and PAUSED will not do these
things.
I will give it a shoot :)
>
> 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();
> }
>
Understood, in this way the implemenation can support the continuation
and no continuation environment at the same time.
As we get the control of the interceptor chain, it can be done by
hacking the ServiceInvokerInterceptor and put the response back, but it
will break the old continuation semantic which can make the user method
be called again when the continuation resume is called.
Willem
> 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
>
Re: CXF continuation enhancement
Posted by Daniel Kulp <dk...@apache.org>.
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
Re: CXF continuation enhancement
Posted by Willem Jiang <wi...@gmail.com>.
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 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