Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

why wait for this ssh to finnish? #53

Closed
DrupaListo-com opened this issue Jul 22, 2016 · 8 comments
Closed

why wait for this ssh to finnish? #53

DrupaListo-com opened this issue Jul 22, 2016 · 8 comments

Comments

@DrupaListo-com
Copy link

hi,
I am working on making osync sync faster.

Thus I have a question:

why this around line 2020 in osync.sh:


$SSH_CMD ERROR_ALERT=0 sync_on_changes=$sync_on_changes _SILENT=$_SILENT _DEBUG=$_DEBUG _DRYRUN=$_DRYRUN _VERBOSE=$_VERBOSE COMMAND_SUDO=$COMMAND_SUDO FILE_LIST="$(EscapeSpaces "$TARGET_STATE_DIR/$deleted_list_file")" REPLICA_DIR="$(EscapeSpaces "$replica_dir")" DELETE_DIR="$(EscapeSpaces "$deletion_dir")" FAILED_DELETE_LIST="$(EscapeSpaces "${TARGET[1]}${TARGET[3]}/$deleted_failed_list_file")" 'bash -s' << 'ENDSSH' > "$RUN_DIR/$PROGRAM.remote_deletion.$SCRIPT_PID" 2>&1 &


runs with & = run in background?

LAter on line 2124 there is a "stupid"
sleep 5


I think if we remove the &, then we can remove the sleep too?
Where am I mistaken?

PS. most probably there is sth that made you do that, I think with some ssh options we can un-make you do that waiting ...

Cheers.

@DrupaListo-com
Copy link
Author

PS2. By removing the & + sleep 5 I observe no problems in test script runs. What bad could happen?

PS2. This made the run time here go down from 16s to 11s ( duh )
PS3. I shaved another 3-4 seconds by forcing SLEEP_TIME from 0.1 to 0.05

Both 0.1 and 0.05 are supported on my OS and since 0.05 does not give me too much CPU load more, I tried it. And since you call that sleep 0.1/0.05 many many times due to the WaitForCompletion function, getting the sleep time low shaves as much run time...

In result now my sync runs in 11s instead of in 19/23s...

My end goal if possible is total run time <1s...

@DrupaListo-com
Copy link
Author

PS4. 3s of times above are due to compass compile I put in LOCAL_RUN_BEFORE_CMD, so current run time here is around 7-9s.

@deajan
Copy link
Owner

deajan commented Jul 22, 2016

Actually the & was added to keep time control over the execution of the heredoc command.
I had to add sleep 5 because when finished, it takes a like a second more to complete the transfer of $FAILED_DELETE_LIST.

With the time, I've moved all this into _delete_remote function which is time controlled by it's parent function.
So it should be safe to remove & + sleep 5.
I still will have to make some tests to be sure failed remote deletions get listed correctly when removing sleep 5.

As for SLEEP_TIME, I need to have a value that works on all OS. If I happen to test it on BSD/Mac/Win/Linux, then I'll lower it to it's minimal value.

End goal < 1s can't be achieved because of ssh connection speed I think when remote syncing.

I'll have everything sorted out next week as I really would love to release v1.1 final soon if no blockers are found.

@DrupaListo-com
Copy link
Author

about the <1s goal - yeah - multiple like 5-10 separate ssh+rsync calls with 0.5/1s each give around 5-10s alone + other stuff = around 10/15s run time...

We could try finding a way to adjust the sleep time based on OS type - and/or make it settable in options... Lets leave it to rest and think about it later...

I am thinking maybe all these sleeps and waits can be abandonned in favour of some more elegant waiting mechanism... I need to grasp the whole idea in using these waiting functions - maybe you need the flow to be synchronous at times and async in other places ?

Other place for improvement is if it was possible to decrease the separate ssh calls... Needs more work...

@deajan
Copy link
Owner

deajan commented Jul 25, 2016

Mainly, all the sync functions must be called sequentially in order to support resuming.
All the lock / unlock / check disk space / misc functions can be parrallelized but should only bring a small amount of speed improvement as local functions are executed quite fast.

I will release v1.1 without the speed improvements as they require deeper code modifications.
This week I'll fork to v1.2-dev, first test builds will be available by the end of the week. Feel free to give me any other idea that might speed up the execution.

@deajan
Copy link
Owner

deajan commented Jul 27, 2016

I've commited a quick and dirty concept. It lowers all the sleep times and adds parallelism to non essential functions.
As there is more code, it should run slower on very small sync sets (some hundred milliseconds), but should run faster the bigger the sync set is.

Can you try it ?

@deajan
Copy link
Owner

deajan commented Jul 27, 2016

Also, lowering SLEEP_TIME too much will trade exeuction time for more CPU usage, I think .05 is a good value.

@deajan
Copy link
Owner

deajan commented Aug 29, 2016

Tested removing sleep time with large 45M fileset. Okay.

@deajan deajan closed this as completed Aug 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants