Unison dual-sync feature `unox` for efficient syncing and with user/id mapping #64

mickaelperrin · 2016-07-21T17:09:47Z

While working on #42, I learned a lot of things about unison, some were already pointed partially by contributors (@gagarine and @onnimonni, thanks for making me think about that).

There is no absoluty need to run unison with root and use --user and --numericids. Running unison under the user we want is sufficient.
Unison is very efficient when used with the option -repeat watch. It follows events in realtime and update its index in realtime while performing sync. When used in conjunction with fsevents, it needs to scan all folder on each request to update it's index.
There is no need to run a server on each side. A simple call of unison with -socket can handle both sync and watch on both sides.

Point 2 and 3 are true, if the system is compatible with unison-fsmonitor. On Mac, you need to install unox to get it working.

This PR introduces :

a new sync strategy `unison-unox``
a new watch strategy unison
an example

The adequate unison image is currently here. I won't push a PR for docker-unison because it is so much simpler than the actual image. Feel free to grab it under your Github account.

I am not sure of the updates of the execution.rb and sync_manager.rb. I wasn't able to get unison working as a new thread, I needed to fork it. This is a problem because, the .join method no longer prevents the main process to exit if you use only this strategy. I use a silly hack with sleep, sure there is better implementation than that. But my 1 week experience with ruby prevent me to find a better way ;)

Hope we will get some good results with this new strategy, until the OSXFS #42 approach will be patched on the Docker side.

- PB: while the command launched has no errors, the sync/watch doesn't work. - FIX: it looks like that creating a new process instead of a thread resolves the issue. - TODO: ensure that this is the adequate way to resolve the issue

- PB: docker-sync exit because of the lack of threads. Indeed, unison is launched as fork. So forked unison processes are not killed when docker-sync ends. - FIX: sleep indefinitely if there are no threads

EugenMayer · 2016-07-21T17:21:04Z

Wow, that looks very interesting - thank you for outlining your think-process and way to get this running.

I would love to take some time to have a look at this, to see what you are going to.

Its interesting to see that this is a different go on #42 due to the issues you outlined with OSXFS and events after some time.

Do you already have any benchmarks with unox vs fswatch on lets say 15k files?

Thank you for your huge effort!

mickaelperrin · 2016-07-21T17:29:06Z

This is the only approach with magic-sync that allowed me to scaffold a full Drupal project with 15k in realtime and with reasonable CPU usage.

Unison dualside failed with this test and I needed to deactivate syncing to use it while bootstrapping.

I ask me even if this is not faster than magic (if we haven't the osxfs issue) as we rely on both side on native file systems.

EugenMayer · 2016-07-21T17:31:46Z

So basically unison-dualside fails due to CPU usage on bigger projects ( deal breaker for you and me )
And magic fails for now due to the buggy implementation of OSXFS

And this is the third angle to tackle the dual-sync side. Nice progress with those real-case tests

mickaelperrin · 2016-07-21T19:04:56Z

Sorry, I was on the go while I answered your latest message.

To be precise, dualside works with heavy projects, but doesn't support their scaffolding. That's why I implemented watch_stop / watch_start. It helped me to bootstrap with a script like the one provided on the wiki.

I would love to see unox have a simple run and forget workflow. I will update this PR with my observations while using it.

Would love to have your opinions too and especially on the ruby implementation of the process management.

EugenMayer · 2016-07-21T19:06:40Z

Sure, i will. Will have to probably take on this on the weekend though

mickaelperrin · 2016-07-21T22:15:08Z

Added a new feature that removes the annoying config sync_host_port. Now, host port is discovered automatically.

Yet, I don't like parsing the output of cli commands. Maybe, we should think about using a ruby docker api, like docker-api ?

EugenMayer · 2016-07-21T22:22:34Z

lib/docker-sync/sync_strategy/unison-unox.rb

@@ -105,7 +106,7 @@ def start_container
          exists = `docker ps --filter "status=exited" --filter "name=#{container_name}" | grep #{container_name}`
          if exists == ''
            say_status 'ok', "creating #{container_name} container", :white if @options['verbose']
-            cmd = "docker run -p '#{@options['sync_host_port']}:#{UNISON_CONTAINER_PORT}' \
+            cmd = "docker run -p '127.0.0.1::#{UNISON_CONTAINER_PORT}' \


looks like 2x :: only one needed

This is intended, I don't specify the host port that should be on the middle of the pattern ip:host_port:container_port

ah, i see, sorry

EugenMayer · 2016-07-21T22:23:48Z

i think what you did there is practical, i would not yet pull in a other library ( for now ) if we have more reasons for it, i would follow you on this one. What do you think?

EugenMayer · 2016-07-22T13:37:23Z

We probably want to thnk about https://github.com/mauricioklein/docker-compose-api since it better parses and utilizes the compose-file and lets us check status / start things by segments. This could make a lot of sense. Did not think deeply into it, pasted for gathering informations

EugenMayer · 2016-07-23T14:41:49Z

lib/docker-sync/sync_manager.rb

        end

+        sleep unless has_threads


why you want to design this blocking by design?

As I said previously in the first message of the PR.

I wasn't able to get unison working as a new thread, I needed to fork it. This is a problem because, the .join method no longer prevents the main process to exit if you use only this strategy. I use a silly hack with sleep, sure there is better implementation than that.

If you don't sleep, the main process exits and you have no way to kill started unison process automatically.

well we have to start thinking if we start working with pid / pid files whatever solutions are common in ruby.

EugenMayer · 2016-07-23T14:47:03Z

For now it seems like you pulled way to many changes into this features. You replaced the way threadexec works https://github.com/mickaelperrin/docker-sync/blob/feature/unox/lib/docker-sync/execution.rb#L24 which needs testing for itself. I would suggest to make this a own PR we can test isolated since this will probably influence all fswatch or at least, ensure fswatch still works like desired.

In general, lets try to now blow this out of proportion or we will have heard time to get it all together. It seems like you try to already implement the "daemon" mode here , mentioned in #51

To be reviewed / documented:

new join_threads + sleep in there
new forkexec
WatchStrategy::Unison::initialize -> why is a sync strategy started here? Is this the "host watcher?"

In general could you document what you did in SyncStrategy::Unison_Unox so what is started what for, why which arguments - so we have some knowledge sharing and can maintain this easier in the future ( or other contributors ). Thanks!

mickaelperrin · 2016-07-24T12:12:35Z

~~Well spotted for the threadexec, this is a bad commit. This function doesn't have to change in any way. I will update this.~~

After reviewing the code, this wasn't a bad commit indeed. I simply factorized the common parts between the threadexec and forkexec functions. To exhibits that they are the same functions, except one use Thread.new and the other Process.fork. I will remove the factorization.

mickaelperrin · 2016-07-26T12:51:44Z

:) This is only unox related indeed.

But, my question is indeed more related to general ruby why a process launched in ruby doesn't perform similarly as a process launched from cli ? should I take attention of some thing in particular.

onnimonni · 2016-07-26T12:53:33Z

Could it be that the ruby process doesn't have unox in it's $PATH?

EugenMayer · 2016-07-26T12:53:52Z

lets split of the general question then into a special issue, what do you think? The reason for this issue could be on different layers, we have to investigate. ENV like path could be a cause

mickaelperrin · 2016-07-26T13:04:34Z

lib/docker-sync/sync_strategy/unison-unox.rb

+        end
+        begin
+          Preconditions::unison_available
+          Preconditions::unox_available


@onnimonni thanks for trying to help, but I have implemented a precondition to check that unox is available. Or maybe this check is broken ?

- PB: when run with popen3 unison stopped to cath file events suddently. - FIX: using backticks did the trick. - TODO: understand why this happens

mickaelperrin · 2016-07-26T14:07:47Z

Using backticks instead of popen3did the trick. Now, need to understand why...

EugenMayer · 2016-07-26T15:12:41Z

You probably want to have a look at http://i.stack.imgur.com/1Vuvp.png ..

So the main differences between popen3 and backticks are, popen is non blocking, backticks are blocking.

The change you just made will not make it into the master branch, since this once again influences fswatch behavior. Please either create a customer execution or solve the issue a different way. We wont stderr/out to be captured - so i suggest you revert that

mickaelperrin · 2016-07-26T15:19:10Z

I agree, I wish it has a "better" solution. But to be precise:

this won't influence fswatch as this is implemented in the function forkexec that it called with only this watch strategy
stdout is captured, as indicated in your graphic. If we need stderr, I think adding 2&>1 to the command will help.

EugenMayer · 2016-07-26T15:23:11Z

oh sorry, got confused, my bad. I guess we will have to solve a couple of those cli-invocation issues somehow, e.g. a issue with process #69 or #65 . Probably a total different approach as you suggest either kind of a process manager or somewhat different. The issue will be, not to "overcomplicate" things.

I do not have a slighly idea why backticks work, popen do not. I those cases, it was always the ENV being different or one of the commands is pickier about what is needed to be escaped?

mickaelperrin · 2016-07-26T15:33:12Z

I checked that using backticks still ensure that processes are killed when we kill the main process. But maybe, in some circumstances, we will have some zombie processes as you get with fswatch in #69 .

Regarding implementation as a daemon #69 , I think it would be a good enhancement. But to be honest, I need less this feature since the use of unox. As it performs well so far, I let it run even when doing heavy I/O. If something goes wrong, kill and restart correct synchronisation issues.

However from my point of view both #69 and #65 are independent of this PR at the moment. But, yes this PR will need to be updated if we change someting more globally.

EugenMayer · 2016-07-26T15:35:27Z

agreed, independent but it could solve issues occurring here probably, thats why i referenced here, so we could probably keep track of the progress. I just expect that we go different routes solving similar issues and see, what solutions works out best

mickaelperrin · 2016-08-01T21:30:53Z

@EugenMayer I just let you review the latest commits and will merge the branch if it is ok for you.

EugenMayer · 2016-08-02T08:18:56Z

lib/docker-sync/preconditions.rb

+      raise('Could not find unison-fsmonitor binary in path. Please install it, see https://github.com/hnsl/unox, or simply run :
+      wget -O /usr/local/bin/unison-fsmonitor https://raw.githubusercontent.com/hnsl/unox/master/unox.py \
+      && chmod +x /usr/local/bin/unison-fsmonitor')
+    end


@mickaelperrin a pitty there is now brew alternatives. I am thinking about creating a own tap and adding this.

Actually, in this case, use yes? and ask if the command should be run, run it automatically then. This makes the most sense here to ease up the installation

I thought about auto install also, but as you didn't provide autoinstall of unison / fswatch / rsync... I stick with a simple help message on how to install.

The reason was, not everybody has brew. In your case, no dependencys are needed and its the only way to install. AFAIK wget is not part of OSX by default, you might want to use curl.

I will change the others also by the way, adding a question, like for the images

EugenMayer · 2016-08-02T08:20:21Z

@mickaelperrin for my perspective, this looks great. The one thing automating / easing up unox installation with a confirmation would actually help a lot and then just head on mergin it. Great work

EugenMayer · 2016-08-02T08:34:05Z

@mickaelperrin see #80 , we should change back the image name to eugenmayer/unison:unox due to legacy support / pre release support . We chnage it to eugenmayer/unison some releases later, so we are sure no/few are using the older versions.

curl is default on Mac OS X, wget needs MacPorts

EugenMayer · 2016-08-02T16:13:18Z

lib/docker-sync/preconditions.rb

+        `#{cmd}`
+      else
+        raise("Please install it, see https://github.com/hnsl/unox, or simply run :\n #{cmd}")
+      end


mickaelperrin added 8 commits July 21, 2016 16:27

[FEATURE] Sync / watch strategy unison with unox

5238549

[BUGFIX] Unox syncing doesn't work in docker-sync

44f1c16

- PB: while the command launched has no errors, the sync/watch doesn't work. - FIX: it looks like that creating a new process instead of a thread resolves the issue. - TODO: ensure that this is the adequate way to resolve the issue

[BUGFIX] Unison processes are not killed

e0d2c7e

- PB: docker-sync exit because of the lack of threads. Indeed, unison is launched as fork. So forked unison processes are not killed when docker-sync ends. - FIX: sleep indefinitely if there are no threads

[BUGFIX] Remove code from previous PR

979914b

[TASK] Cleanup, remove unused code

21c7925

[FEATURE] Add support for user/uid mapping

7987b24

[FEATURE] Prevent error if uid is already in use

a22ff51

[TASK] Add example for unison-unox

e3bfc11

gagarine mentioned this pull request Jul 21, 2016

Use unox strategie for sync gagarine/docker-php#1

Closed

[FEATURE] Host port discovery

ee9ff46

EugenMayer reviewed Jul 21, 2016
View reviewed changes

mickaelperrin added 2 commits July 22, 2016 00:32

[FEATURE] Set unison as default watcher for unox

4910928

[BUGFIX] sync_host_port is no more checked

a28ec76

mickaelperrin mentioned this pull request Jul 21, 2016

Sync does not work mickaelperrin/docker-magic-sync#1

Closed

mickaelperrin added 3 commits July 22, 2016 01:14

[BUGFIX] sync_host_port is no more checked

2ca5715

[TASK] Remove old code

4d2c7fd

merge

ada42b3

EugenMayer reviewed Jul 23, 2016
View reviewed changes

EugenMayer added the feature label Jul 25, 2016

mickaelperrin reviewed Jul 26, 2016
View reviewed changes

[BUGFIX] Unison doesn't catch file events

e3ecf79

- PB: when run with popen3 unison stopped to cath file events suddently. - FIX: using backticks did the trick. - TODO: understand why this happens

mickaelperrin mentioned this pull request Jul 28, 2016

Unison: conflict resolution #70

Closed

EugenMayer mentioned this pull request Jul 31, 2016

Syncing with unison-dualside is syncing across src directories #71

Closed

mickaelperrin added 5 commits August 1, 2016 21:50

[TASK] Set default image as unison

e94d9e6

[TASK] Info how to install unox in error message

27c5928

[TASK] Merge master and conflict resolution

17688ba

[TASK] Rename unison-unox to unison

224ecaf

[TASK] Update and fix example

1884f1a

EugenMayer reviewed Aug 2, 2016
View reviewed changes

mickaelperrin added 3 commits August 2, 2016 10:42

[TASK] Revert default image to unison:unox

b569ac2

[TASK] Use curl instead of wget in unox install

84baa85

curl is default on Mac OS X, wget needs MacPorts

[FEATURE] Auto install of unox

5f288f2

mickaelperrin force-pushed the feature/unox branch from a4ae725 to 5f288f2 Compare August 2, 2016 10:11

EugenMayer reviewed Aug 2, 2016
View reviewed changes

mickaelperrin merged commit c4e9f21 into EugenMayer:master Aug 2, 2016

zxaos mentioned this pull request Aug 26, 2016

unison-fsmonitor fails to install correctly with stock python but does not warn #128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unison dual-sync feature `unox` for efficient syncing and with user/id mapping #64

Unison dual-sync feature `unox` for efficient syncing and with user/id mapping #64

mickaelperrin commented Jul 21, 2016

EugenMayer commented Jul 21, 2016

mickaelperrin commented Jul 21, 2016

EugenMayer commented Jul 21, 2016

mickaelperrin commented Jul 21, 2016

EugenMayer commented Jul 21, 2016

mickaelperrin commented Jul 21, 2016

EugenMayer Jul 21, 2016

mickaelperrin Jul 21, 2016

EugenMayer Jul 21, 2016

EugenMayer commented Jul 21, 2016

EugenMayer commented Jul 22, 2016

EugenMayer Jul 23, 2016

mickaelperrin Jul 24, 2016

EugenMayer Jul 24, 2016

EugenMayer commented Jul 23, 2016

mickaelperrin commented Jul 24, 2016 •

edited

Loading

mickaelperrin commented Jul 26, 2016

onnimonni commented Jul 26, 2016 •

edited

Loading

EugenMayer commented Jul 26, 2016 •

edited

Loading

mickaelperrin Jul 26, 2016

mickaelperrin commented Jul 26, 2016

EugenMayer commented Jul 26, 2016

mickaelperrin commented Jul 26, 2016

EugenMayer commented Jul 26, 2016

mickaelperrin commented Jul 26, 2016

EugenMayer commented Jul 26, 2016

mickaelperrin commented Aug 1, 2016

EugenMayer Aug 2, 2016

mickaelperrin Aug 2, 2016

EugenMayer Aug 2, 2016

mickaelperrin Aug 2, 2016

EugenMayer commented Aug 2, 2016

EugenMayer commented Aug 2, 2016

EugenMayer Aug 2, 2016

Unison dual-sync feature unox for efficient syncing and with user/id mapping #64

Unison dual-sync feature unox for efficient syncing and with user/id mapping #64

Conversation

mickaelperrin commented Jul 21, 2016

EugenMayer commented Jul 21, 2016

mickaelperrin commented Jul 21, 2016

EugenMayer commented Jul 21, 2016

mickaelperrin commented Jul 21, 2016

EugenMayer commented Jul 21, 2016

mickaelperrin commented Jul 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EugenMayer commented Jul 21, 2016

EugenMayer commented Jul 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EugenMayer commented Jul 23, 2016

mickaelperrin commented Jul 24, 2016 • edited Loading

mickaelperrin commented Jul 26, 2016

onnimonni commented Jul 26, 2016 • edited Loading

EugenMayer commented Jul 26, 2016 • edited Loading

Choose a reason for hiding this comment

mickaelperrin commented Jul 26, 2016

EugenMayer commented Jul 26, 2016

mickaelperrin commented Jul 26, 2016

EugenMayer commented Jul 26, 2016

mickaelperrin commented Jul 26, 2016

EugenMayer commented Jul 26, 2016

mickaelperrin commented Aug 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EugenMayer commented Aug 2, 2016

EugenMayer commented Aug 2, 2016

Choose a reason for hiding this comment

Unison dual-sync feature `unox` for efficient syncing and with user/id mapping #64

Unison dual-sync feature `unox` for efficient syncing and with user/id mapping #64

mickaelperrin commented Jul 24, 2016 •

edited

Loading

onnimonni commented Jul 26, 2016 •

edited

Loading

EugenMayer commented Jul 26, 2016 •

edited

Loading