You are viewing a plain text version of this content. The canonical link for it is here.
Posted to axis-cvs@ws.apache.org by Sanjiva Weerawarana <sa...@opensource.lk> on 2006/09/27 00:52:55 UTC

Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

On Tue, 2006-09-26 at 17:29 +0000, nagy@apache.org wrote:
> Author: nagy
> Date: Tue Sep 26 10:29:14 2006
> New Revision: 450121
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=450121 Log: AXIS2-1233
>  Added a method to the handler interface, and the mechanisms to invoke
>  it, to allow handlers to be called after processing of a particular
>  message flow, on which they took part, has completed.

Hi Bill,

I'm sorry but I'm -1 on this change. This is related to the discussion
we've been having since April IIRC and AFAICT we never concluded that.
We're about to do a 1.1 release and this is not the time to do this kind
of an extremely major changing affecting every downstream consumer and a
big change in the processing model. 

Can you please revert this and let's work on it after we release 1.1.

Thanks,

Sanjiva.


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org


Re: [Axis2] Handler invocation pattern improvements (was Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...)

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
On Wed, 2006-09-27 at 23:01 -0700, Bill Nagy wrote:
> I have backed out my commit as per the -1, and reopened JIRA 1233.
> Could you please now explain why you don't agree with my solution to the
> problem?

Sorry for the slow response Bill.

So Deepal and I met Bill when we were recently in Seattle (yes he's
moved there .. no not to Microsoft ;-)). We concluded that there was no
other way to solve the problem that he's trying to solve. There is some
improvement that can be done by sharing the same list of handlers that
are used for execution and reversing that but that requires a more
risky / deeper change. The performance concern I raised earlier will
become a real concern only when you have lots of handlers .. but
presumably in that case those handlers are doing something useful;
implying they will take execution time. So the extra delay introduced
will be negligable.

So I'm fine for going ahead with this change .. please commit it back in
right after Thilina cuts the branch for the 1.1 release.

Thanks for your patience and apologies for the delays in settling this
issue.

Sanjiva.



---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


[Axis2] Handler invocation pattern improvements (was Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...)

Posted by Bill Nagy <na...@watson.ibm.com>.
I have backed out my commit as per the -1, and reopened JIRA 1233.
Could you please now explain why you don't agree with my solution to the
problem?

-Bill

On Tue, 2006-09-26 at 21:45 -0700, Bill Nagy wrote:
> Hi Sanjiva,
> 
> While the JIRA issue may have had "cleanup" in the title, points 2 and 3
> of the issue (and also Deepal's later comments) deal with what I fixed.
> 
> I'm not sure that I understand why you say "the impl that has been
> committed is going to slow Axis2 down big time."  It makes one extra
> method invocation to each phase, which in turn makes one extra method
> call to each handler that was previously invoked for each message flow.
> If the handlers don't need to do anything, then that time is negligible
> -- if they do need to do something, then they need to do the work, and
> spend the time for a reason.
> 
> -Bill
> 
> 
> On Wed, 2006-09-27 at 09:58 +0530, Sanjiva Weerawarana wrote:
> > Hi Bill,
> > 
> > > This change actually addresses part of a JIRA issue that was marked as a
> > > "blocker" (AXIS2-653), and according to Thilina's post these are to be
> > > fixed before the release.  (I will address the other part as well, but
> > > there were two separate issues so I separated this one out and tackled
> > > if first.)  
> > 
> > I think the original issue which was marked a blocker was that cleanup()
> > wasn't called. That's not what's implemented by this patch. 
> > 
> > I am not opposed to solving the problem you want to solve with this
> > patch but the impl that has been committed is going to slow Axis2 down
> > big time. We have discussed the problem but IIRC didn't come to a
> > consensus on the solution. I'd really rather work thru this more before
> > we do it. If a performance degrading solution is the only one then I'd
> > like to think long and hard about it before doing it. 
> > 
> > We've gotten hammered on the user mailing list for having stuff that is
> > not fully baked in a major release. Doing this now (literally 2 days
> > before the planned code freeze) is going to put in something that hasn't
> > been tried, tested and proven into a major release. 
> > 
> > So, I'd like to request that we remove the blocker status on this issue
> > and revert this patch. My apologies but I'm not lifting my -1 on this
> > patch.
> > 
> > I guess I should be arguing with the release manager for leaving this as
> > a blocker! 
> > 
> > Let's pick it up after 1.1 and sort it out. ApacheCon US would be a
> > great time to do it if you'll be around there.
> > 
> > Sanjiva.
> > 
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
> > For additional commands, e-mail: axis-cvs-help@ws.apache.org
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-cvs-help@ws.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

Posted by Bill Nagy <na...@watson.ibm.com>.
Hi Sanjiva,

While the JIRA issue may have had "cleanup" in the title, points 2 and 3
of the issue (and also Deepal's later comments) deal with what I fixed.

I'm not sure that I understand why you say "the impl that has been
committed is going to slow Axis2 down big time."  It makes one extra
method invocation to each phase, which in turn makes one extra method
call to each handler that was previously invoked for each message flow.
If the handlers don't need to do anything, then that time is negligible
-- if they do need to do something, then they need to do the work, and
spend the time for a reason.

-Bill


On Wed, 2006-09-27 at 09:58 +0530, Sanjiva Weerawarana wrote:
> Hi Bill,
> 
> > This change actually addresses part of a JIRA issue that was marked as a
> > "blocker" (AXIS2-653), and according to Thilina's post these are to be
> > fixed before the release.  (I will address the other part as well, but
> > there were two separate issues so I separated this one out and tackled
> > if first.)  
> 
> I think the original issue which was marked a blocker was that cleanup()
> wasn't called. That's not what's implemented by this patch. 
> 
> I am not opposed to solving the problem you want to solve with this
> patch but the impl that has been committed is going to slow Axis2 down
> big time. We have discussed the problem but IIRC didn't come to a
> consensus on the solution. I'd really rather work thru this more before
> we do it. If a performance degrading solution is the only one then I'd
> like to think long and hard about it before doing it. 
> 
> We've gotten hammered on the user mailing list for having stuff that is
> not fully baked in a major release. Doing this now (literally 2 days
> before the planned code freeze) is going to put in something that hasn't
> been tried, tested and proven into a major release. 
> 
> So, I'd like to request that we remove the blocker status on this issue
> and revert this patch. My apologies but I'm not lifting my -1 on this
> patch.
> 
> I guess I should be arguing with the release manager for leaving this as
> a blocker! 
> 
> Let's pick it up after 1.1 and sort it out. ApacheCon US would be a
> great time to do it if you'll be around there.
> 
> Sanjiva.
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-cvs-help@ws.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org


Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

Posted by Bill Nagy <na...@watson.ibm.com>.
Hi Sanjiva,

While the JIRA issue may have had "cleanup" in the title, points 2 and 3
of the issue (and also Deepal's later comments) deal with what I fixed.

I'm not sure that I understand why you say "the impl that has been
committed is going to slow Axis2 down big time."  It makes one extra
method invocation to each phase, which in turn makes one extra method
call to each handler that was previously invoked for each message flow.
If the handlers don't need to do anything, then that time is negligible
-- if they do need to do something, then they need to do the work, and
spend the time for a reason.

-Bill


On Wed, 2006-09-27 at 09:58 +0530, Sanjiva Weerawarana wrote:
> Hi Bill,
> 
> > This change actually addresses part of a JIRA issue that was marked as a
> > "blocker" (AXIS2-653), and according to Thilina's post these are to be
> > fixed before the release.  (I will address the other part as well, but
> > there were two separate issues so I separated this one out and tackled
> > if first.)  
> 
> I think the original issue which was marked a blocker was that cleanup()
> wasn't called. That's not what's implemented by this patch. 
> 
> I am not opposed to solving the problem you want to solve with this
> patch but the impl that has been committed is going to slow Axis2 down
> big time. We have discussed the problem but IIRC didn't come to a
> consensus on the solution. I'd really rather work thru this more before
> we do it. If a performance degrading solution is the only one then I'd
> like to think long and hard about it before doing it. 
> 
> We've gotten hammered on the user mailing list for having stuff that is
> not fully baked in a major release. Doing this now (literally 2 days
> before the planned code freeze) is going to put in something that hasn't
> been tried, tested and proven into a major release. 
> 
> So, I'd like to request that we remove the blocker status on this issue
> and revert this patch. My apologies but I'm not lifting my -1 on this
> patch.
> 
> I guess I should be arguing with the release manager for leaving this as
> a blocker! 
> 
> Let's pick it up after 1.1 and sort it out. ApacheCon US would be a
> great time to do it if you'll be around there.
> 
> Sanjiva.
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-cvs-help@ws.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org


Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Hi Bill,

> This change actually addresses part of a JIRA issue that was marked as a
> "blocker" (AXIS2-653), and according to Thilina's post these are to be
> fixed before the release.  (I will address the other part as well, but
> there were two separate issues so I separated this one out and tackled
> if first.)  

I think the original issue which was marked a blocker was that cleanup()
wasn't called. That's not what's implemented by this patch. 

I am not opposed to solving the problem you want to solve with this
patch but the impl that has been committed is going to slow Axis2 down
big time. We have discussed the problem but IIRC didn't come to a
consensus on the solution. I'd really rather work thru this more before
we do it. If a performance degrading solution is the only one then I'd
like to think long and hard about it before doing it. 

We've gotten hammered on the user mailing list for having stuff that is
not fully baked in a major release. Doing this now (literally 2 days
before the planned code freeze) is going to put in something that hasn't
been tried, tested and proven into a major release. 

So, I'd like to request that we remove the blocker status on this issue
and revert this patch. My apologies but I'm not lifting my -1 on this
patch.

I guess I should be arguing with the release manager for leaving this as
a blocker! 

Let's pick it up after 1.1 and sort it out. ApacheCon US would be a
great time to do it if you'll be around there.

Sanjiva.



---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org


Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Hi Bill,

> This change actually addresses part of a JIRA issue that was marked as a
> "blocker" (AXIS2-653), and according to Thilina's post these are to be
> fixed before the release.  (I will address the other part as well, but
> there were two separate issues so I separated this one out and tackled
> if first.)  

I think the original issue which was marked a blocker was that cleanup()
wasn't called. That's not what's implemented by this patch. 

I am not opposed to solving the problem you want to solve with this
patch but the impl that has been committed is going to slow Axis2 down
big time. We have discussed the problem but IIRC didn't come to a
consensus on the solution. I'd really rather work thru this more before
we do it. If a performance degrading solution is the only one then I'd
like to think long and hard about it before doing it. 

We've gotten hammered on the user mailing list for having stuff that is
not fully baked in a major release. Doing this now (literally 2 days
before the planned code freeze) is going to put in something that hasn't
been tried, tested and proven into a major release. 

So, I'd like to request that we remove the blocker status on this issue
and revert this patch. My apologies but I'm not lifting my -1 on this
patch.

I guess I should be arguing with the release manager for leaving this as
a blocker! 

Let's pick it up after 1.1 and sort it out. ApacheCon US would be a
great time to do it if you'll be around there.

Sanjiva.



---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org


Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

Posted by Bill Nagy <na...@watson.ibm.com>.
Hi Sanjiva,

This change actually addresses part of a JIRA issue that was marked as a
"blocker" (AXIS2-653), and according to Thilina's post these are to be
fixed before the release.  (I will address the other part as well, but
there were two separate issues so I separated this one out and tackled
if first.)  

-Bill
   
On Wed, 2006-09-27 at 04:22 +0530, Sanjiva Weerawarana wrote:
> On Tue, 2006-09-26 at 17:29 +0000, nagy@apache.org wrote:
> > Author: nagy
> > Date: Tue Sep 26 10:29:14 2006
> > New Revision: 450121
> > 
> > URL: http://svn.apache.org/viewvc?view=rev&rev=450121 Log: AXIS2-1233
> >  Added a method to the handler interface, and the mechanisms to invoke
> >  it, to allow handlers to be called after processing of a particular
> >  message flow, on which they took part, has completed.
> 
> Hi Bill,
> 
> I'm sorry but I'm -1 on this change. This is related to the discussion
> we've been having since April IIRC and AFAICT we never concluded that.
> We're about to do a 1.1 release and this is not the time to do this kind
> of an extremely major changing affecting every downstream consumer and a
> big change in the processing model. 
> 
> Can you please revert this and let's work on it after we release 1.1.
> 
> Thanks,
> 
> Sanjiva.


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org


Re: svn commit: r450121 - in /webservices/axis2/trunk/java/modules: integration/test/org/apache/axis2/engine/ kernel/src/org/apache/axis2/context/ kernel/src/org/apache/axis2/engine/ kernel/src/org/apache/axis2/handlers/ kernel/src/org/apache/axis2/transpo...

Posted by Bill Nagy <na...@watson.ibm.com>.
Hi Sanjiva,

This change actually addresses part of a JIRA issue that was marked as a
"blocker" (AXIS2-653), and according to Thilina's post these are to be
fixed before the release.  (I will address the other part as well, but
there were two separate issues so I separated this one out and tackled
if first.)  

-Bill
   
On Wed, 2006-09-27 at 04:22 +0530, Sanjiva Weerawarana wrote:
> On Tue, 2006-09-26 at 17:29 +0000, nagy@apache.org wrote:
> > Author: nagy
> > Date: Tue Sep 26 10:29:14 2006
> > New Revision: 450121
> > 
> > URL: http://svn.apache.org/viewvc?view=rev&rev=450121 Log: AXIS2-1233
> >  Added a method to the handler interface, and the mechanisms to invoke
> >  it, to allow handlers to be called after processing of a particular
> >  message flow, on which they took part, has completed.
> 
> Hi Bill,
> 
> I'm sorry but I'm -1 on this change. This is related to the discussion
> we've been having since April IIRC and AFAICT we never concluded that.
> We're about to do a 1.1 release and this is not the time to do this kind
> of an extremely major changing affecting every downstream consumer and a
> big change in the processing model. 
> 
> Can you please revert this and let's work on it after we release 1.1.
> 
> Thanks,
> 
> Sanjiva.


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-cvs-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-cvs-help@ws.apache.org