tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shinta Tjio <st...@broadjump.com>
Subject RE: FW: problem w/ ajp13 - if Tomcat is shutdown
Date Wed, 14 Mar 2001 21:15:18 GMT
Henri, Dan,
There are some discussions below about the usage
of msg/rmsg. What did you guys finally decide
on that? Just reuse msg? I'm seeing the problem
Dan mentioned on message larger than the ajp13
buffer (8K).

thanks,
shinta

> -----Original Message-----
> From: GOMEZ Henri [mailto:hgomez@slib.fr]
> Sent: Tuesday, March 13, 2001 4:49 PM
> To: tomcat-dev@jakarta.apache.org
> Cc: danmil@shore.net
> Subject: RE: FW: problem w/ ajp13 - if Tomcat is shutdown
> 
> 
> > 1) First off, I strongly believe that this work (which I think is an
> >excellent feature, BTW), belongs in the 3.3 branch, and 
> *not* in 3.2. 
> 
> +1, there was a message about including latest mod_jk 
> features/corrections
> back to Tomcat 3.2.x but TC 3.2 Release Manager and others clairly
> stated that TC 3.2.x must be just correction of bugs without any
> new features. I asked about back-porting mod_jk/ajp13 since now
> Apache 2.0 or upload works fine with mod_jk/ajp from TC 3.3 branches
> but are not in TC 3.2. The decision was not to provide corrections
> to TC 3.2.
>  
> >Although it feels like a bug that we need to restart Apache 
> >every time we restart TC, it's documented behavior, and I 
> >consider fixing it more of a feature enhancement.  
> >The scope of work you're doing here is considerable,
> >and the code you're modifying is complex (and involves lots of 
> >different possible situations).  I would not feel 
> comfortable committing 
> >this to 3.2 unless it had seen a *lot* of testing in the 3.3 
> branch.  (Am 
> >I incorrect in remembering that there has been discussion of 
> putting this
> into 3.2?)
> 
> Right many works, some testing but back-port could introduce 
> bugs in TC3.2.
> The decision was to not port mod_jk back to tc 3.2. But 
> people could still
> build mod_jk from Tomcat 3.3 and use against there Tomcat 
> 3.2.x. That's why
> I milite for a jakarta sub-project (web-connector). These 
> kind of connector
> work must outside Tomcat trees. (Votes ?)
> 
> > 2) Specific problems (line numbers by patch against the 3.3 
> code base)
> > 
> > - msg/rmsg: Here is how I think you want the code to work: 
> >msg always holds the basic request information, so, in case 
> of an error, we
> can 
> >resend the message.  rmsg is used as the buffer for response 
> information.
> 
> Dan you're our resident hacker/expert on mod_jk native part 
> 
> >This is a good idea, but there are a few problems -- first, and most
> >importantly, if there is POST data, the "msg" buffer 
> >immediately gets reused to send that information to TC (ll. 
> 596-600).  
> >In that case,  on line 691, if you retry after a connection 
> reset event, 
> >you'd be using a buffer filled with POST data instead of a 
> buffer with
> header information.  
> 
> I'm not too confortable with msg/rmsg that's why I asked for your code
> revue (and it's excellent ;-)
> 
> >Possibly you could fix this by calling 
> ajp13_marshal_into_msgb again when 
> >you retry -- I don't know how the state of the 
> web_server_service object
> (s), 
> >changes over time, so I don't know for sure if this would 
> work or not.  But
> 
> >that's where I'd look next.  
> 
> >Or you could pass rmsg into the sendrequest, and use that to
> >send the POST data over.  I dunno.
> 
> 
> >Your use of msg/rmsg in getreply seems faulty to me.  I don't 
> >think you need msg there at all, and I'm sure that you've 
> got a problem
> with 
> >mixing the two of them.  Specifically, on l. 636, you call 
> >
> >  rc = ajp13_process_callback(rmsg, p, s, l);
> >
> >But, then a few lines later, you do:
> >
> >  else if(JK_AJP13_HAS_RESPONSE == rc) {
> >	rc = connection_tcp_send_message(p, msg, l);
> 
> Right, we must drop rmsg use and do all stuff with msg.
> 
> 
> >If you look at ajp13_process_callback, at the GET_BODY_CHUNK 
> >case, you'll see that it's reading request data into the 
> buffer passed in
> as a param
> >(which is what getreply is calling 'rmsg').  But then, when you call
> >connection_tcp_send_message, you'll be sending over whatever 
> >was in msg, rather than what the web server has read from 
> the browser.  
> 
> >You can fix this by changing connection_tcp_send_message to 
> use 'rmsg' (and
> 
> >then you don't need to pass msg into getreply at all).
> 
> >This problem will happen in case of a file upload, or 
> whenever there is
> >enough POST data to exceed a single ajp13 buffer (8K).  
> 
> >Furthermore, in that
> >case, I'm not sure if there's going to be any way to restart 
> >the connection
> >intelligently.  If the browser has already sent over 500 K 
> of a 1M file
> >upload when TC dies, I think we need to just kill that 
> >request, rather than
> >trying to restart it. I don't think the request is 
> >recoverable. I'm not sure
> >how to detect this in your code, but I think it needs to be 
> >thought about.
> 
> Upload are a real problem since we just couldn't replay the upload
> in case of Tomcat failure. Solution could be to ask browser to
> resend but HOW (HTTP expert needed here)
> 
> > - The for loop on l. 689 makes me nervous.  Could you rewrite that:
> >int i;
> >for(i = 0 ; i < JK_RETRIES ; i++)
> >That way you won't get an infinite loop if JK_RETRIES < 0.  
> >And I think it's
> >more idiomatic.  But, hey, that's up to you ;-).
> 
> +1
> 
> > - Do you understand the JK_METHOD macro?  I don't fully, but 
> >take a look in
> >jk_global.h.  In Win32 and Netware environments, this macro is set to
> >"__stdcall" -- is this only needed for things which are called 
> >from outside
> >this file (like service())?  If neither of us understand it, 
> 
> JK_METHOD (stdcall) is needed when used you use functions
> from outside but in our case it's static and so it's safe
> to not set the JK_METHOD for sendrequest/getreply.
> 
> >I'd like to
> >have people on Win32 and Netware systems test this (maybe they 
> >could even
> >explain why that call is needed -- and then we could document 
> >it!)  Maybe
> >you should be using it with sendrequest and getreply?
> 
> > - A few logging inconsistencies -- you call jk_log(l, 
> >JK_LOG_DEBUG,...),
> >when I think you want JK_LOG_ERROR.  Check l. 642, 702. 
> 
> +1, 
> 
> > - Error/log message inconsistencies: 
> >   - l. 632 You mean "error reading response", I think
> >   - l. 642 You have "1" where you want ":", I think.
> >
> > - You seem to be using 4-character tabs -- the rest of the 
> >C-code (AFAIK) uses 8-character tabs.  I think that's our standard 
> >(it is on  the Java side, for sure).  In any event, this is 
> >making your new code render bizarrely in my editor.
> 
> I read sometimes ago there was a early decision on TAB length
> by Apache team (httpd) but didn't remember what the final decision.
> Jon Stevens are to remove all TAB and replace by space, I'll take
> a look at native code and set my editor accordinly (vi/ue)
> 
> > - Style notes (I'm style obsessive, I admit).  I would make it
> >ajp13_send_request (or at least send_request) and ajp13_get_reply (or
> >get_reply).
> 
> Style is important, I'll do that.
> 
> I still have problem accessing jakarta-tomcat from CVS in anonymous
> mode, but I'll try with my jakarta login. I'll rework the patch 
> with (or without) rmsg use and resend for study ;-)
> 
> Thanks for your analyze.
> 
> >-Dan
> >
> >GOMEZ Henri wrote:
> >> 
> >> >In the mean time, I have taken Henri's changes and back
> >> >port it to 3.2.1 (because I need it on 3.2.1). Everything
> >> >seems to work well. I've tested it in the normal scenarios
> >> >(one Apache, one Tomcat) and in the load-balanced scenarios.
> >> 
> >> Thanks for using the patch and tested it under LoadBalanced
> >> Tomcats.
> >> 
> >> >In the load-balanced scenarios, when I restart TC worker 1,
> >> >the code properly close the dead sockets and re-establish
> >> >new ones to the same worker (TC worker 1). The good
> >> >connections to TC worker 2 are untouched. They stay
> >> >connected.
> >> 
> >> Normal procedure
> >> 
> >> >I did notice something wierd. But this is un-related
> >> >to the code edits. This happens with or without Henri's
> >> >changes. When I restart TC worker 1, but shut down TC
> >> >worker 2, requests that supposed to go to TC worker 2
> >> >(because they belong to the same session, thus the load
> >> >balancer try to foward it to the same TC worker 2) took
> >> >sometime to get forwarded to TC worker 1. This maybe
> >> >another one of those "improvements" that can be done
> >> >to the load balancer worker.
> >> 
> >>  No problem here, when you shutdown a Tomcat in a
> >> load balancing architecture, you got request goes
> >> to the second even if there is a JVMROUTE set .
> >> 
> >> >Anyway, I'm pretty happy with Henri's changes. (Thanks
> >> >Henri!). Henri, are you going to check in the changes?
> >> 
> >> I'd like to see Dan check it since I created a second memory
> >> pool rmsg but I'm not too confident on it :
> >> 
> >> +        rmsg = jk_b_new(&(p->pool));
> >> +        jk_b_set_buffer_size( rmsg, DEF_BUFFER_SZ);
> >> +     jk_b_reset(rmsg);
> >> 
> >> See you
> >> 
> >> 
> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> >> For additional commands, email: tomcat-dev-help@jakarta.apache.org
> >
> >-- 
> >
> >Dan Milstein // danmil@shore.net
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> >For additional commands, email: tomcat-dev-help@jakarta.apache.org
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, email: tomcat-dev-help@jakarta.apache.org
> 

Mime
View raw message