3

I am running the below MERGE query against my Neo4j server from a client application in 10 parallel threads, the newFoo and id parameters are the same on all 10 runs:

MERGE (foo:Foo { id: {id} })
ON MATCH
SET foo = {newFoo}

After running this, I run the below query to expect 1 but I instead get 10:

match (f:Foo)
return count(f)

I thought that MERGE runs in an atomic transaction but apparently not. Am I doing something wrong here?

Update

Below is the code that I used to reproduce the issue:

public static async Task RunInParallel()
{
    var client = new GraphClient(new Uri("http://localhost:7474/db/data"), "neo4j", "1234567890")
    {
        JsonContractResolver = new CamelCasePropertyNamesContractResolver()
    };

    client.Connect();

    var foo = new Foo
    {
        Id = "1",
        Name = "Foo",
        Slug = "foo-bar-foo"
    };

    List<Task> tasks = new List<Task>();
    for (int i = 0; i < 10; i++)
    {
        var task = client.Cypher
            .Merge("(foo:Foo { id: {id} })")
            .OnMatch()
            .Set("foo = {newFoo}")
            .WithParams(new
            {
                Id = foo.Id,
                NewFoo = foo
            })
            .ExecuteWithoutResultsAsync();

        tasks.Add(task);
    }

    await Task.WhenAll(tasks.ToArray());
}
tugberk
  • 57,477
  • 67
  • 243
  • 335
  • 1
    For the record, the `newFoo` and `id` parameters are actually not the same. The `newFoo` value (after camel case resolution) is `{id: "1", name: "Foo", slug: "foo-bar-foo"}`. The `id` parameter value is just `"1"`. – cybersam Dec 16 '15 at 01:39
  • you also have different spellings for your id-property, once with capitalized I and once with lowercase i – Michael Hunger Dec 16 '15 at 20:11
  • If you just have 2 additional params. 1. use `ON CREATE `!!! 2. use `SET foo.name = {newFoo}.name, foo.slug = {newfoo}.slug` – Michael Hunger Dec 16 '15 at 20:12
  • @MichaelHunger yes but `CamelCasePropertyNamesContractResolver` on mky client settings should handle that. – tugberk Dec 16 '15 at 21:40

1 Answers1

7

MERGE (by itself) does not guarantee uniqueness. When using MERGE (on a unique property) you should always create a uniqueness constraint for the specified property:

CREATE CONSTRAINT ON (f:Foo) ASSERT f.id IS UNIQUE

This will ensure you aren't creating any duplicates.

Edit

MERGE without a uniqueness constraint is not thread safe. Adding a uniqueness constraint ensures an index lock is held before writing, making the operation thread safe.

William Lyon
  • 8,371
  • 1
  • 17
  • 22
  • Well, adding a unique constraint made it a bit worse, getting deadlock errors: `"DeadlockDetectedException: LockClient[31] can't wait on resource RWLock[INDEX_ENTRY(49), hash=2045111578] since => LockClient[31] <-[:HELD_BY]- RWLock[NODE(980), hash=779731960] <-[:WAITING_FOR]- LockClient[25] <-[:HELD_BY]- RWLock[INDEX_ENTRY(49), hash=2045111578]"` – tugberk Dec 16 '15 at 01:20
  • If MERGE in this use-case doesn't guarantee uniqueness, then the documentation should be changed (http://neo4j.com/docs/stable/query-merge.html). Nothing in that writeup can mean anything other than uniqueness guaranteed. "The MERGE clause ensures that a pattern exists in the graph. Either the pattern already exists, or it needs to be created." In this case, if this is expected behavior, it should end with, "or it exists and gets re-created anyway." – David Fox Dec 16 '15 at 17:01
  • 1
    @tugberk adding a uniqueness constraint ensures that Neo4j grabs a lock when using MERGE - thus the deadlock exception with highly concurrent writes. Try catching the DeadlockException and implementing a retry loop. More info here: http://neo4j.com/docs/stable/transactions-deadlocks.html – William Lyon Dec 16 '15 at 17:16
  • 2
    @DavidFox thanks for the feedback. I will pass that along. Using MERGE without a uniqueness constraint is not thread safe. Adding a uniqueness constraint ensures thread safety for MERGE by using an index lock. – William Lyon Dec 16 '15 at 17:21
  • 2
    Thanks @WilliamLyon. Some reflection of that in the docs would definitely be useful. I've had similar issues to this before and to me it seems like the concepts of locking in Neo4j can be documented in a much better way. Also, I'm not too familiar with the nitty-gritty of the locking system, but to me it seems odd that just simple MERGE queries would ever get deadlocked because the whole point of MERGE is to create or return/update an already-existing entity. – David Fox Dec 16 '15 at 18:07
  • 1
    @WilliamLyon this is a bit unfortunate and the word `MERGE` completely threw me off here. The fact that is being called `MERGE`and has its current behaviour is so confusing. I would expect `MERGE` to handle locking silently, create the resource in an Atomic fashion and hand me the node reference. Otherwise, it's no different than `CREATE` with on a node which has a unique constraint (if I am not mistaken). – tugberk Dec 16 '15 at 21:39