Bug 140750 - Fish uploads files very slowly
Fish uploads files very slowly
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: mc (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jindrich Novy
: FutureFeature
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2004-11-24 12:21 EST by Dmitry Butskoy
Modified: 2013-07-02 19:03 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2004-12-13 05:08:10 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Appropriate patch, fixing slow FISH uploads (1.72 KB, patch)
2004-11-24 12:24 EST, Dmitry Butskoy
no flags Details | Diff
Possible enhancement of the previous patch -- better fallback (1.30 KB, patch)
2004-11-30 11:37 EST, Dmitry Butskoy
no flags Details | Diff
Final patch (2.83 KB, patch)
2004-12-08 09:30 EST, Dmitry Butskoy
no flags Details | Diff

  None (edit)
Description Dmitry Butskoy 2004-11-24 12:21:39 EST
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3)
Gecko/20040923

Description of problem:

  Midnight Commander has a FISH virtual filesystem (FIle transfer over
SHell). There is one old problem in MC implementation of it -- uploads
onto remote system.
  To do such an upload, MC should invoke on the remote side a command,
which reads appropriate number of bytes from stdin and store them in
the target file. Initially, `dd' command had been chosen for this purpose.
                                                                     
          
  The decision was such:
" ( dd bs=4096 count=<size/4096> ; dd bs=<size%4096> count=1 ) | ( cat
>target_file ; cat >/dev/null ) "
  (Note: additional cat to /dev/null is needed to flush input on write
errors).
  But unfortunately, this variant appeared unreliable. The design of
`dd' does not assume full filling of input buffers on read (see, for
example, "conv=sync" description in `dd' manual). The design of `ssh'
is those that sometimes data can be written into pipe by portions of
different size. Therefore the part of data could remain not read by
`dd' ...

  In the current version, the problem is solved as:
" dd bs=1 count=<size> | ..... "
i.e., `dd' reads input byte-by-byte. It is robast, but is very slow
and grabs a lot of cpu time on the remote system.
                                                                     
          

Version-Release number of selected component (if applicable):
mc-4.6.1-0.9

How reproducible:
Always

Steps to Reproduce:
Just see the source.
Or try to upload something big (several megabytes)...

Additional info:


  The better decision is possible.

  There is `head' command. It has `-c <number>' option, which does
what we really need. The necessary amount of data is read from stdin
reliably.
  Unfortunately, the remote system (any *NIX-like) may not have `head'
command, or have another implementation of it. But for such systems we
can still use `dd' .
                                                                     
          
  The final decision is:
" ( head -c <size> -q - || dd bs=1 count=<size> ) | ( cat >target_file
; cat >/dev/null ) "
  Either fast `head' is used, or `dd' as a fallback.
                                                                     
          
  If there is no `head' on the remote side, or `head' is too old (has
no `-c' option), then `dd' will be used. `-q' and `-' are used to
cause most non-GNU `head's to fail (by "incorrect" option). We can not
trust non-GNU implementations of `head' -- for example, AIX has `-c'
option, but always adds extra newline at the end of target file :-( .
There is small probability that remote `head' is not GNU head but
supports all our options. Let`s believe that it is "modern" head and
it suits us! :-)
                                                                     
          
  The new variant of FISH uploads is successfully tested by us in
various environments (GNU-like, non-GNU) -- all works as it is expected.
                                                                     
          
  Appropriate patch is provided.
Comment 1 Dmitry Butskoy 2004-11-24 12:24:29 EST
Created attachment 107407 [details]
Appropriate patch, fixing slow FISH uploads
Comment 2 Leonard den Ottolander 2004-11-24 13:22:34 EST
Dmitry,

Please be so kind to report this issue upstream, at mc-devel@gnome.org.
Comment 3 Jindrich Novy 2004-11-25 04:16:43 EST
Hello Dmitry,

thanks for the RFE. It seems like the fish improvement you presented
is good at least for the mc here in Red Hat since GNU head is used
here and the behaviour of head on AIX sounds more like a bug than a
feature to me. I see no problem with possible inclusion because of
possible fallback to dd when things goes wrong with head.
Comment 4 Dmitry Butskoy 2004-11-30 11:37:24 EST
Created attachment 107632 [details]
Possible enhancement of the previous patch -- better fallback

  Possible, I have found a better way for fallback. It is several times faster
than initial "dd bs=1 count=%lu" ...
  The idea is we use shell loop as this:

while :
do
    set @@@ `ls -l <filename>`
    [ $6 -lt <size> ] && dd bs=4096 count=1 || break
done

  Note: `@@@' is ised to avoid "option-like" interpretation of `ls' output.

  Each invokation `dd' reads only once ("count=1"), with maximum amount of 4096
bytes. Actually, reading occurs in the smaller portions - as `ssh' put data in
the pipe.
  We never read the extra data because MC do not give us anything more until we
echo status code to it.

  This way successfully tested by us in the GNU/non-GNU environment.
Comment 5 Leonard den Ottolander 2004-11-30 11:50:36 EST
Again, you better discuss this upstream. It might take a while before
your patch gets applied as people seem to be quite busy lately, but
showing your face upstream might have a faster result. It's just that
people tend to pay more attention to people they've seen around before
and that they trust. And in general it doesn't hurt to draw attention
to your patch once more (as long as you avoid nagging ;) ).
Comment 6 Dmitry Butskoy 2004-12-06 07:45:45 EST
Comment on attachment 107632 [details]
Possible enhancement of the previous patch -- better fallback

It was more a draft, rather than a patch. It was posted for discussion purpose.
Continue in upstream...

  Initial patch (head || dd) is OK, anyway :-)
Comment 7 Leonard den Ottolander 2004-12-06 07:55:09 EST
Sadly head is not portable. This was mentioned to me by Pavel
Shirshov, I'm not sure if he mailed that to mc-devel.
Comment 8 Jindrich Novy 2004-12-06 08:03:27 EST
I don't think the patch is obsolete for the Red Hat version of mc
since the portability to i.e. to FreeBSD is not so important here.
This is the right argument to modify the patch upstream though.
Comment 9 Dmitry Butskoy 2004-12-06 08:07:40 EST
> Sadly head is not portable

Yes, I wrote about it! Therefore `dd' is still used, but us a fallback.

  "head" only is not portable, but "head || dd" seems to be.
Comment 10 Dmitry Butskoy 2004-12-06 08:14:54 EST
  Jindrich,

  My "better fallback" patch, as it was posted, does not handle disk
overflow case and does not work with file append. 
  More fast fallback, IMHO, is possible, but this requires more
complex shell script...
  Anyway, may be apply the initial idea ("head || dd") and then think
about better fallbacks?
Comment 11 Jindrich Novy 2004-12-06 08:45:03 EST
Ok, that makes more sence to me now. Please try to comment here when
you set a patch as obsolete. Any fixes from you are welcome.

The reason that I haven't committed this patch yet is that I think we
should find a better way to speed up fish transfers even without the
head. When the patch is modified in such a way it will be worth to
send also to upstream.
Comment 12 Dmitry Butskoy 2004-12-08 09:30:00 EST
Created attachment 108110 [details]
Final patch

  I have found the decision. IMHO, it is both fast and robast.

  See the comments in the patch.

  I have tested both variants (head and fallback) when remote machine is under
Linux, and fallback variant with AIX4 .
  I also have checked GNU and some old (1984) UNIX sources of all utilities
used - all should be OK.
Comment 13 Jindrich Novy 2004-12-08 09:51:29 EST
Could you please send this patch and comment also to upstream?
(mc-devel@gnome.org)
Comment 14 Dmitry Butskoy 2004-12-08 11:37:06 EST
Sent upstream.
  But MC team seem to be too busy. Could you make some tests too?
  
Comment 16 Leonard den Ottolander 2004-12-10 17:16:23 EST
Dmitry,

The mc team has been very active in the last few months. Indeed people
seem to be busy at the moment. But we have been very responsive in
general, so you should show some patience. Please take this business
back upstream. I think the problem is not many people use fish, which
is why your patches might not get tested immediately.

And as a general rule, if you want people to test your patches you
might want to test some patches from others in return.
Comment 17 Jindrich Novy 2004-12-13 05:08:10 EST
Dmitry, your patch is now applied in the latest development release,
mc-4.6.1a-0.1, so that we have more people to test it now.
Comment 18 Leonard den Ottolander 2004-12-14 16:40:38 EST
Jindrich,

This versioning will cause you a headache. Why not just continue with
4.6.1-0.10?

Once you will release 4.6.1-1 it will appear older than 4.6.1a-0.1 :( .

And don't you pull an epoch stunt on us ;) !

Personally I do use characters on versions, but only on the last
number of the subversion. Please retract this version and tell those
who installed it to "downgrade" to a new release with a sensible
version number.
Comment 19 Dmitry Butskoy 2005-06-16 07:44:14 EDT
Jindrich,

As this patch has already appeared outside rawhide (after FC4 had been
released), may be try to send it to mainstream once again?
I would prefer that it was made by you as you have more influences in MC team.
Comment 20 Jindrich Novy 2005-06-29 09:38:52 EDT
Dmitry,

it seems that the fish speedups works well as I can see no bugreports for this
since it was applied. I'm aware of no response to your post to upstream mc. I'll
try to ping mc upstream developers about this, but maybe the right time is after
the upcoming mc release to not to delay it.

Note You need to log in before you can comment on or make changes to this bug.