search

OSC / ood-fileexplorer

[MOVED] The Open OnDemand File Explorer
https://osc.github.io/Open-OnDemand/
MIT License
4 stars 1 forks source link

rsync for copy with progress reporting #202

Closed MorganRodgers closed 5 years ago

MorganRodgers commented 5 years ago

Attempting to replace copymitter with Rsync while still retaining the ability to update the UI with progress messages. I do not know why but progress messages are being consumed by the Nginx error log instead of being sent to the client. I am using the same method-I thought-to update the client as copymitter was using.

Two commits of interest:

Unless we are able to get the messages out of the logs and to the client this branch offers no improvements over use_rsync_instead_of_copymitter.

After additional testing it appears to me that the defect is present in the production version as well. In production the first copy operation after page-load never shows progress. Subsequent copies actions do show progress messages.

Knowing that the defect exists in master I have opened a new issue: https://github.com/OSC/ood-fileexplorer/issues/204 and am declaring this branch a success.

ericfranz commented 5 years ago

So, I know collectively a non-trivial amount of time was put into the rsync solution... but...

what about using https://www.npmjs.com/package/fs-extra instead?

Since it utilizes the base node.js fs library perhaps it is a safer option than requiring a new external dependency such as rsync. Since it would be best to make a patch release with this bug fix and release sooner than later...

I came across fs-extra from this discussion https://stackoverflow.com/questions/13786160/copy-folder-recursively-in-node-js. I do not know anything else beyond that, or benefits or drawbacks to the current solution, and have not closely read the code to consider how it is actually doing what it does.

MorganRodgers commented 5 years ago

We can certainly try it. I'm curious why copymitter fails in the first place as all it is doing is reading the file stream and writing it back out at the new location: https://github.com/coderaiser/node-copymitter/blob/v1.8.13/lib/copymitter.js#L216-L220. fs-extra is using fs.copy: https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/copy.js#L96. I don't know if the Node standard lib item is more robust on Lustre.

MorganRodgers commented 5 years ago

While working on the software requirements docs I realized that we already require rsync on the systems to use the Job Composer, and so using it here does not introduce a new dependency per se.

MorganRodgers commented 5 years ago

A strike against this implementation is the version of rsync available by default on Centos 6 (version 3.0.6 protocol version 30) does not have the --info argument that we are using to implement progress messages.

ericfranz commented 5 years ago

We ended up disabling cloud commander's progress updating, which utilizes a web socket connection to facilitate, in #208.

The HTTP request initiated copy works, the WS initiated copy fails on Luster. By disabling progress updating we reduce the code paths to just the HTTP request and ensure that works on Lustre and other file systems. We will add progress tracking as a requirement for the files app replacement.

Since we disabled progress tracking, this fix is no longer applicable.