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:[EMAIL PROTECTED]]
> Sent: Tuesday, March 13, 2001 4:49 PM
> To: [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED]
> 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: [EMAIL PROTECTED]
> >> For additional commands, email: [EMAIL PROTECTED]
> >
> >--
> >
> >Dan Milstein // [EMAIL PROTECTED]
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: [EMAIL PROTECTED]
> >For additional commands, email: [EMAIL PROTECTED]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]
>
