Understanding when to use Atomic Property Updates

[deepak]

I need some help figuring out if my use-case requires atomic updates (using APOC) or a property setter is enough. My doubt arises because all the write queries in my Java WebApp use an auto-commit transaction with no retries. I have a node that keeps track of counts for all child and grand-child nodes

(A)<-[:Child]-(B)<-[:Child]-(C)<-[:Child]-(D) (all relations are many-many)

 A has properties countB, countC, countD that's incremented/decremented when new children are added. So, only one node is being updated for my use-case. I am not sure if multiple instances/threads running this query can possibly overwrite each others' changes, or they are guaranteed to run sequentially since this statement runs in a transaction without depending on any other nodes. Assume newCount* variables are user parameters to Cypher -

MATCH(a:A{id:1234})
SET a.countB = a.countB + newCountB
SET a.countC = a.countC + newCountC
SET a.countD = a.countD + newCountD
RETURN a

If the cypher above is buggy, will APOC updates fix the problem? Do APOC atomic procedures acquire a different lock from what SET statements do?

MATCH(a:A{id:1234})
CALL apoc.atomic.add(a,'countB',newCountB,5)
YIELD oldValue, newValue
CALL apoc.atomic.add(a,'countC',newCountC,5)
YIELD oldValue, newValue
CALL apoc.atomic.add(a,'countD',newCountD,5)
YIELD oldValue, newValue
RETURN a

How does one test such scenarios?

[glilienfield]

They way I understand it is, if you have a SET for a property, then the node gets locked for the duration of the transaction. I once wrote a test to verify this by creating a large number of tasks that updated the property on the same node and submitted them to a thread pool.  The result was that the updates were done sequentially, so the node was locked during updating, effectively serializing the operations. I pulled the unit test out and included it here if you want to verify.  

I don't think you APOC implementation will work, as each individual update is atomic, but the set of three is not. 

import com.xCrafter.itemService.configurations.Neo4jInMemoryTestConfig;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.neo4j.driver.Driver;
import org.neo4j.driver.Result;
import org.neo4j.driver.Session;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

import static org.assertj.core.api.Assertions.assertThat;
import static org.neo4j.driver.Values.parameters;

@ContextConfiguration(classes = Neo4jInMemoryTestConfig.class,
        initializers = ConfigDataApplicationContextInitializer.class)
@ExtendWith(SpringExtension.class)
public class TestConcurrency {

    private static final ExecutorService threadPool = Executors.newFixedThreadPool(50);

    @Autowired
    private Driver driver;

    @Test
    public void testConcurrency() {
        List<Long> listOfIds = new ArrayList<>();
        try (final Session session = driver.session()) {
            Callable<Long> callable = new Callable<>() {
                @Override
                public Long call() throws Exception {
                    return session.writeTransaction(tx -> {
                        Result result = tx.run(
                                "MERGE (id:UniqueId {name:'Invoice'}) " +
                                        "ON CREATE SET id.id = 1200 " +
                                        "ON MATCH SET id.id = id.id+1 " +
                                        "RETURN id.id as id");
                        return result.single().get("id").asLong();
                    });
                }
            };
            for (int i = 0; i < 1000; i++) {
                Future<Long> future = threadPool.submit(callable);
                Long id = future.get();
                listOfIds.add(id);
            }
            System.out.println("ids: " + listOfIds);
        } catch (ExecutionException e) {
            e.printStackTrace();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

[deepak]

Thanks @glilienfield for confirming with tests, sounds reassuring.

This raises another question - when should we use the apoc.atomic.* procedures if SET auto-locks the nodes enforcing sequentiality.

[glilienfield]

Sorry, that question I can't answer. 

Just a note, using transaction functions is no more difficult than auto-commit transactions. Whatever you are doing now can be wrapped in a transaction function. 

[glilienfield]

NOTE...I just noticed the above code is incorrect. I foolishly submitted the callable to the executor and waited for it to complete with the immediate 'get', before submitting another tasks.  As such, the executor tasks are serialized, so I am not demonstrating what I wanted to. So please ignore the conclusion I made about the SET operation locking the node. I need to do further investigation. 

[deepak]

@michael_hunger Do you mind helping on this? I see you were the pioneer behind the atomic procedures.

From the github conversation, it seems SET does not acquire a lock during updates, and therefore it's apoc.atomic.* makes more sense for my use-case.