concurrency errors with dolt_merge

[wupeaking]

I checkout many branches from dev branch. then I insert some data on different brances. when I merge these branch to dev, it occurs some errs.

I upload the code demo. demo.go.zip

[fulghum]

Hey @wupeaking, thanks for reporting this and for giving us some repro code. We'll jump on this today and let you know what we find.

[jennifersp]

Hi @wupeaking, thank you for providing the demo. There are couple things need fixing in the demo:

• The first merge performs fast-forward, which there is nothing to commit, so the dolt_commit() after will cause an error of no changes were made. You can avoid that by using --allow-empty flag.
• The second merge will be successful, but it will leave the working set dirty. As the third merge tries to merge, it will cause an error from dirty working set because the second merge has not committed yet. Here is a repro of this case: demo.go.zip

Can you give us more details on your use case, so we can help see if there is a better way to do it and avoid the race problem in merges.

[zachmu]

Thanks for drawing this to our attention.

I'm digging in a bit more, and I think there may actually be a bug here. I'll let you know what I find.

In the meantime, you should be able to work around this by doing your own locking. You need to ensure that only one client is merging into the dev branch at a time. You can do this in your application code with mutexes, e.g.:

mu.Lock()
_, err = newTx.Exec(fmt.Sprintf("call dolt_merge('%s')", newBranch))
_, err = newTx.Exec(fmt.Sprintf("call dolt_commit('--allow-empty', '-am', 'merge from %s')", newBranch))
mu.Unlock()

Or you can let the database do the locking for you with the get_lock() and release_lock functions, documented here:

https://dev.mysql.com/doc/refman/8.0/en/locking-functions.html

We're also reconsidering whether merge should create a new commit by default, rather than requiring a separate dolt_commit call. We're tracking that here:

https://github.com/dolthub/dolt/issues/3773

[zachmu]

OK, on closer inspection, dolt_merge() is definitely not going to handle real concurrency without some work on our end. For now our recommendation is to dolt_merge() with only one thread at a time. Make your locks align with transaction boundaries, i.e.:

mu.Lock()
newTx, _ := db.Beginx()
_, err = newTx.Exec(fmt.Sprintf("call dolt_merge('%s')", newBranch))
_, err = newTx.Exec(fmt.Sprintf("call dolt_commit('--allow-empty', '-am', 'merge from %s')", newBranch))
// no need to commit transaction here, dolt_commit() commits the transaction for you
mu.Unlock()

Thanks again for doing this testing. We'll keep you up to date on solutions to this problem. Our goal is that your original code should run correctly without any explicit locking or modification.

[wupeaking]

@zachmu @jennifersp Thanks for fixing the demo code. In fact, I will run multiple web apps to perform similar merge operations. At present, it is not possible to perform concurrent operations in multiple services like traditional databases.

[timsehn]

All other concurrent transactions work as traditional databases. You just have to single thread dolt_merge().

[zachmu]

If you're running these merge operations from multiple machines, you'll want to lock the branch being merged using database locks, like this:

newTx, _ := db.Beginx()
_, err = newTx.Exec(fmt.Sprintf("SELECT GET_LOCK('%s')", newBranch))
_, err = newTx.Exec(fmt.Sprintf("call dolt_merge('%s')", newBranch))
_, err = newTx.Exec(fmt.Sprintf("call dolt_commit('--allow-empty', '-am', 'merge from %s')", newBranch))
_, err = newTx.Exec(fmt.Sprintf("SELECT RELEASE_LOCK('%s')", newBranch))
// no need to commit transaction here, dolt_commit() commits the transaction for you
mu.Unlock()

We'll have an improved concurrency story for this use case soon. But until then this is probably your best bet.

[wupeaking]

@zachmu Thank you very much! I'll try it. 😄

[timsehn]

I'm going to resolve because we gave you a workaround.