Re: cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

[Takashi Yano <takashi.yano@nifty.ne.jp>]

On Mon, 6 Sep 2021 14:49:55 +0200
Corinna Vinschen wrote:
> On Sep  6 20:16, Takashi Yano wrote:
> > Hi Corinna,
> > 
> > On Mon, 6 Sep 2021 10:13:10 +0200
> > Corinna Vinschen wrote:
> > > If you have multiple readers, all but one of them will hang in this
> > > WFSO.  They will block here without a chance to kill or Ctrl-C them
> > > and thread cancellation won't work.
> > > 
> > > To fix that you have to use cygwait and handle signals and thread
> > > cancellation the same way as in the below code following the NtReadFile.
> > 
> > OK. Thanks for the advice. Then what about the patch attached?
> > 
> > > >  	  /* If the pipe is empty, don't request more bytes than half the
> > > > -	     pipe buffer size.  Every pending read lowers WriteQuotaAvailable
> > > > -	     on the write side and thus affects select's ability to return
> > > > -	     more or less reliable info whether a write succeeds or not.
> > > > -
> > > > -	     Let the size of the request depend on the number of readers
> > > > -	     at the time. */
> > > > +	     pipe buffer size. Pending read lowers WriteQuotaAvailable on
> > > > +	     the write side and thus affects select's ability to return
> > > > +	     more or less reliable info whether a write succeeds or not. */
> > > > +	  ULONG chunk = max_atomic_write / 2;
> > > >  	  status = NtQueryInformationFile (get_handle (), &io,
> > > >  					   &fpli, sizeof (fpli),
> > > >  					   FilePipeLocalInformation);
> > > > -	  if (NT_SUCCESS (status) && fpli.ReadDataAvailable == 0)
> > > 
> > > If the readers are serialized anyway, why fetch only half the remaining
> > > buffer size?  In that case fetching fpli.InboundQuota - 1 is as good
> > > as fetching just the half of it, isn't it?
> > 
> > It sounds reasonable. Adopted in the attached patch.
> 
> Patch looks ok.
> 
> I added one more patches:
> 
> - It occured to me that the code is lacking a CancelIo in case cygwait
>   is waiting for NtReadFile/NtWriteFile.  Actually, calling cygwait
>   without "mask" parameter will result in cygwait performing the thread
>   cancellation by itself, but cancelling a thread does not cancel the
>   async IO started by that thread.  So I fixed the cygwait calls in
>   raw_read/raw_write to return to the caller and then call CancelIo
>   before cancelling the thread.
> 
> I planned to push one more patch:
> 
> - Drop max_atomic_write, it's == DEFAULT_PIPEBUFSIZE anyway
> 
> But then some things were coming to mind, which we still have to discuss.
> 
> - I think setting chunk to DEFAULT_PIPEBUFSIZE - 1 in the read case and
>   DEFAULT_PIPEBUFSIZE in the write case by default is dangerous.
>   Assuming the pipe has been created by a non-Cygwin process, the values
>   may be way too high.
> 
>   Suggestion: Actually set max_atomic_write to something useful.
>   Set max_atomic_write to DEFAULT_PIPEBUFSIZE in fhandler_pipe::create.
>   In case of stdio handles inherited from non-Cygwin processes, fetch
>   the pipe buffer size via NtQueryInformationFile in
>   dtable::init_std_file_from_handle().  Better, in a matching
>   fhandler_pipe method called from init_std_file_from_handle().
> 
> - What about calling select for writing on pipes read by non-Cygwin
>   processes?  In that case, we still can't rely on WriteQuotaAvailable,
>   just as before.
> 
>   I have a vague idea that we might want to count readers in that case,
>   but I have to think about it some more.

Current git head seems to have some bug. With and without my patch,
sftp get for large file causes error:

[yano@Express5800-S70 ~]$ sftp 192.168.0.133
yano@192.168.0.133's password:
Connected to 192.168.0.133.
sftp> get test.dat
Fetching /home/yano/test.dat to test.dat
test.dat                                       13%   66MB  66.4MB/s   00:06 ETAReceived message too long 1728053256
Ensure the remote shell produces no output for non-interactive sessions.
[yano@Express5800-S70 ~]$ sftp 192.168.0.133
yano@192.168.0.133's password:
Connected to 192.168.0.133.
sftp> get test.dat
Fetching /home/yano/test.dat to test.dat
test.dat                                       22%  111MB 110.6MB/s   00:03 ETAdo_download: parse: incomplete message
[yano@Express5800-S70 ~]$



-- 
Takashi Yano <takashi.yano@nifty.ne.jp>