Recreate mutagen sync session when mutagen.yml changes, fixes #4753 #4837
Conversation
|
Download the artifacts for this pull request:
See Testing a PR |
There was a problem hiding this comment.
Hey @rfay, I had a look over this and I think the strategy is good; similar to what Mutagen Compose does (Mutagen Compose actually does an in-memory configuration comparison, but the result is the same). I only had one comment and it may be something that is already addressed elsewhere in the code.
| @@ -976,12 +976,20 @@ Fix with 'ddev config global --required-docker-compose-version="" --use-docker-c | |||
|
|
|||
| if app.IsMutagenEnabled() { | |||
| if ok, volumeExists, info := CheckMutagenVolumeSyncCompatibility(app); !ok { | |||
| util.Debug("mutagen sync session and docker volume are in incompatible status: '%s', Removing mutagen sync session '%s' and docker volume %s", info, MutagenSyncName(app.Name), GetMutagenVolumeName(app)) | |||
| util.Debug("mutagen sync session, configuration, and docker volume are in incompatible status: '%s', Removing mutagen sync session '%s' and docker volume %s", info, MutagenSyncName(app.Name), GetMutagenVolumeName(app)) | |||
| terminateErr := TerminateMutagenSync(app) | |||
There was a problem hiding this comment.
I think my only comment would be that it might be worth flushing the stale session before terminating it, that way both endpoints are theoretically the same when the new session is created. Doing that will help ensure that the first reconciliation pass of the new synchronization session will construct a virtual ancestor that's (ideally) equal to both endpoints (and that way no pending unsynced changes are lost and converted to conflicts when the new empty ancestor is created in the new session).
However, my guess is that a flush may be done elsewhere, just before CheckMutagenVolumeSyncCompatibility (and potentially TerminateMutagenSync) is invoked, so if that's the case then just ignore this comment.
Mutagen should probably eventually add a mechanism to transfer the virtual ancestor from one session to a new one (maybe a mutagen sync update command, or something). There are some complexities with that, but it should be possible someday.
There was a problem hiding this comment.
Thanks so much, will check!
rfay
force-pushed
the
20230419_mutagen_check_yml
branch
2 times, most recently
from
64adf28
to
3f6b835
Compare
rfay
force-pushed
the
20230419_mutagen_check_yml
branch
from
3f6b835
to
63ddf57
Compare
The Issue
A sync session can outlive the mutagen.yml file, as it's not checked again after creation
How This PR Solves The Issue
Manual Testing Instructions
ddev startand review the mutagen sync identifier (ddev mutagen st -l | grep Identifier)ddev restartand verify that you have a new mutagen sync session (ddev mutagen sync st -l | grep Identifier)ddev restartwithout changes and verify you have the same session identifier nowAutomated Testing Overview
Added TestMutagenConfigChange and TestFileHash
Release/Deployment Notes
ddev mutagen reseton the new release.