Bug 140750
| Summary: | Fish uploads files very slowly | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Dmitry Butskoy <dmitry> | ||||||||
| Component: | mc | Assignee: | Jindrich Novy <jnovy> | ||||||||
| Status: | CLOSED RAWHIDE | QA Contact: | |||||||||
| Severity: | medium | Docs Contact: | |||||||||
| Priority: | medium | ||||||||||
| Version: | rawhide | CC: | dgunchev, leonard-rh-bugzilla, pknirsch | ||||||||
| Target Milestone: | --- | Keywords: | FutureFeature | ||||||||
| Target Release: | --- | ||||||||||
| Hardware: | All | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Fixed In Version: | Doc Type: | Enhancement | |||||||||
| Doc Text: | Story Points: | --- | |||||||||
| Clone Of: | Environment: | ||||||||||
| Last Closed: | 2004-12-13 10:08:10 UTC | Type: | --- | ||||||||
| Regression: | --- | Mount Type: | --- | ||||||||
| Documentation: | --- | CRM: | |||||||||
| Verified Versions: | Category: | --- | |||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||
| Embargoed: | |||||||||||
| Attachments: |
|
||||||||||
Created attachment 107407 [details]
Appropriate patch, fixing slow FISH uploads
Dmitry, Please be so kind to report this issue upstream, at mc-devel. 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. 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.
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 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 :-)
Sadly head is not portable. This was mentioned to me by Pavel Shirshov, I'm not sure if he mailed that to mc-devel. 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. > 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.
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?
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. 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.
Could you please send this patch and comment also to upstream? (mc-devel) Sent upstream. But MC team seem to be too busy. Could you make some tests too? 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. 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. 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. 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. 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. |
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.