From: Davi Arnaut
Date: January 28 2009 2:14am
Subject: Re: bzr commit into mysql-6.0-backup branch (ingo.struewing:2749)
 Bug#37780
List-Archive: http://lists.mysql.com/commits/64225
Message-Id: <497FBF80.9000502@Sun.COM>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 8BIT

On 1/27/09 1:46 PM, Ingo Strüwing wrote:
> Hi Davi,
>
> Davi Arnaut, 26.01.2009 19:30:
>
>> On 1/21/09 3:05 PM, Ingo Strüwing wrote:
> ...
>>> Davi Arnaut, 20.01.2009 21:22:
>>>
>>> ...
>>>> On 1/11/09 4:05 PM, Ingo Struewing wrote:
> ...
>>> Yes. Keeping the write direction open means that the client can still
>>> receive data. The worst thing that can happen is that the client gets an
>>> error on an attempt to write to the socket. If it interprets this as
>>> "Lost connection to server", then we've won anyway. Just an error
>>> message from the server might get lost.
>> My doubt is about a client polling his socket and bailing out in case
>> poll returns a error if one of the directions is closed. This needs to
>> be documented as a protocol change.

Scratch this, I was wondering about something else.

>
> Ok. Documenting the half-close in KILL situations is surely a good
> thing. However, i don't know where to look for the protocol
> documentation. Can you help out?
>
> OTOH we would need a "broken" implementation of the Berkeley sockets. A
> reasonably conforming implementation would not take notice of the close
> of the read direction on the peer. Unless it tries to actually write to
> the socket. A poll on the write direction will only notice the
> transition from a full write buffer to a non-full write buffer.
>
> Anyway, if a poll misinterprets the half-close as connection failure of
> some kind, it could try to work around it, or close by itself. In the
> former case the full close follows soon. Finally the closed state is
> reached in any case. The only problem might be wrong error messages.
>

Have you tested? Out of curiosity, I looked at Linux's kernel IPv4 
shutdown function (tcp_shutdown) and it actually does nothing if the 
receiving side (SHUT_RD) is to be closed. I also went on to test on OS X 
and writes will succeed even if the receive operations on the server are 
disabled. SHUT_RD only seems to be meaningful on pipes and alike. Is 
this really worth the effort?

>>> I would like to keep adding sync points as easy as possible. If
>>> debug_sync.h is included in mysql_priv.h, it is available in most files.
>>> No extra check for "#include<debug_sync.h>" needs to be done after
>>> adding a sync point. Since debug_sync.h has protection against double
>>> use, I'll leave the explicit uses in the files where you added them.
>> Please, don't add it back to mysql_priv.h.. easy of use is not a concern
>> here.
>
>
> This might be a misunderstanding. I do not want to add back the
> definitions. But just an #include<debug_sync.h>. As I proposed in my patch.

The end result is the same.

> If this is, what you thought, but still object, please explain, what the
> problem is.

First of all, adding debug_sync.h to mysql_priv.h as to ease the 
addition of sync point is not a acceptable argument as including 
debug_sync.h when necessary is a no-brainier. Also, headers should be 
self sufficient and minimal and adding more stuff to mysql_priv.h goes 
against our modularization efforts, slows down compile time, needlessly 
tempers with compiler caches and goes against common coding standards.

I object and I'm pretty sure the second reviewer will object too.

> ...
>>>>> +    /* purecov: begin inspected */
>>>>>         put_error(NULL);
>>>>>         free_defaults(defaults_argv);
>>>>> +    batch_readline_end(status.line_buff);
>>>>>         my_end(0);
>>>>>         exit(1);
>>>>> +    /* purecov: end */
>>>> Unrelated, please commit separated so it can be pushed early.
>>>
>>> Reporting a bug for this, and waiting for it to become tagged by triage,
>>> may finally take longer than to have it included here.
>> OK to push separately. I didn't suggest a separate bug report.
>
>
> Ok, will do. But please note that in former time it was a rule not to
> make patches that are neither related to a bug nor a worklog.

This sends shivers down my spine..

Regards,

-- Davi