Red Hat Bugzilla – Full Text Bug Listing
|Summary:||Fish uploads files very slowly|
|Product:||[Fedora] Fedora||Reporter:||Dmitry Butskoy <dmitry>|
|Component:||mc||Assignee:||Jindrich Novy <jnovy>|
|Status:||CLOSED RAWHIDE||QA Contact:|
|Version:||rawhide||CC:||dgunchev, leonard-rh-bugzilla, pknirsch|
|Fixed In Version:||Doc Type:||Enhancement|
|Doc Text:||Story Points:||---|
|Last Closed:||2004-12-13 05:08:10 EST||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
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 email@example.com.
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? (firstname.lastname@example.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.