Henri,

> You're right. I strongly didn't like to see that mixed use of buffer for
> both input and output. We have now system with Mb of RAM and trying to
> save some Kb is a pitty. We must use at least 2 differents buffers :
> 
> One buffer apache => tomcat , another tomcat => apache

That would be fine, but you'd actually need a third to hold the initial
request data (since the apache => tomcat buffer can otherwise get
overwritten by POST data -- even if there is no file upload).  That could
actually be nice -- that way you could hold onto the POST data in the apache
=> tomcat buffer, and you could do a safe restart if there was < 8K of POST
data / file upload.
 
> >Will this be fully restartable?  I'm not absolutely sure -- it
> >all depends
> >on whether or not we can reread the POST data from Apache
> >after a restart
> >(actually, from *all* of the servers supported by mod_jk).
> 
> We must'nt assume that. In my actual patch by line 633 you'll
> see a canrecover var that I set to true when seeing a upload
> mode. In that case I consider the situation to be unrecoverable.
> The solution is in that case to save all upload datas to file
> and in case of error, redo from that file.

Yes, but you're not setting that when there is POST data -- and it sounds
like either a) you need a third buffer as above or b) you need to give up on
requests with POST data.  You would have to pass this back out of
send_request (since that is where the POST data gets sent over).  I incline
towards (a).
 
> I'm not sure that in case of large upload (ie > Mb) Apache store
> data from browser somewhere. Did HTTP allow us to reask to browser
> to restart the upload ?

I just looked through the HTTP spec (RFC 2616), and the File Upload spec
(RFC 1867), and I'm not seeing anything which would help us.  There is a
"503 Service Unavailable" return status code, but I would be astounded if
most browsers handled this correctly (i.e. they pay attention to the
"Retry-After" header).  We could just send 503 with a Retry-After: of 1
second, and maybe it would work.  I dunno.

I feel okay with saying that, if the client is in the middle of a file
upload or a large POST, they just get a server error (we are restarting TC,
after all -- that's going to generate some error conditions).
 
> > 2) is_recoverable_error is passed into send_request, and it's value is
> >assigned to, but it is in no way checked/used.  Was this also
> >true of the
> >old jk_ajp13 code?  (I believe so, but I'm not sure).  If it's
> >genuinely not
> >being used, we should remove it, but I would like to understand what it
> >should be doing, and make sure we're not missing an error
> >state somewhere.
> 
> It is checked at line 737. If false we stop there.

Yes, but, it is reset to JK_TRUE on 732.  Which is *after* send_request is
called.  So, if send_request sets is_recoverable_error to JK_FALSE, it is
prompty reset to JK_TRUE, get_reply is called, and only then is it checked
on line 737.  So whatever send_request is doing with is_recoverable_error is
being totally ignored.  This is what worries me.
 
> > 4) Still using 4-space tabs.  A quick look through some of the other C
> >classes suggests that most of them don't use tabs at all (just
> >spaces).
> >Let's try to emulate that -- I'll try to stop using 8-space tabs.
> 
> Yes, I'm not confortable with 4/8 tab switch between java and C. But not
> the 'core business' ;-)

No worries -- I just fixed my .emacs so that it always uses spaces.  I feel
very virtuous now ;-).
 
> I'll modify jp_ajp13_worker.c and resend diff. After that one week of
> test (you too ?) and a commit after if nothing is broken.

If I have time to test, I will.  I don't think I'll have a lot of time,
unfortunately.

If you fix the buffer issue and can figure out what is_recoverable_error
should be doing in send_request, I'd be happy to see it merged in.  I would
check with Larry, though -- I'm not sure how he would feel about a new
feature going into 3.3 at this point (as the Release Manager).  This is sort
of hairy code we're playing with here.  It would be great to see it in 3.3,
but I would just check with him first.

-Dan

p.s. Can I mention again how great it is that you're working on this?  Not
having to constantly restart Apache will make me a happy, happy man.

-- 

Dan Milstein // [EMAIL PROTECTED]

Reply via email to