Hi Ran,

Each thread creates 2 nodes in the same transaction. The chain of events
leading to this seems to be the following:

* Thread 1 creates the first node. By the time it leaves createNode(), it
will have a write lock on the ID generator node.
* Thread 2 creates the first node. It will try to update the ID generator
node, but will wait until thread 1 commits the transaction. It's waiting
inside a synchronized context, locking on _lock.
* Thread 1 creates the second node. It waits for _lock to become available.

Now you have a deadlock. The reason that this deadlock is not detected by
Neo4j is because it mixes Neo4j and Java synchronization, which is
impossible for Neo4j to detect.

This can be fixed by replacing the Java synchronization with Neo4j
synchronization. Instead of doing a synchronized block, first grab a write
lock on the ID generator node by e.g. removing a non-existent property. The
thread that gets that lock will then complete its entire transaction before
any other thread gets in there.

In some scenarios this may effectively serialize your transactions. To
avoid this, you can split your transactions in two, where you first
allocate the required number of IDs, and commit that transaction with the
ID generator node only. Then you enter the second transaction where you
actually use the IDs. The tradeoff here is that you may end up allocating
IDs that don't get used, e.g. due to machine crashes at the wrong time.

David

On Wed, Nov 2, 2011 at 12:55 AM, Cres <[email protected]> wrote:

> (Sorry for re-opening this thread, I hadn't yet subscribed to the mailing
> list when I opened it the previous time, so I had to re-start it..)
>
> Hi,
> I'm using Neo4J version 1.4.1. I've tried following the design guide (
> http://wiki.neo4j.org/content/Design_Guide#Make_use_of_Factories )
> writing a
> short program, but when I try to run it, it goes into a deadlock and the
> application freezes.
>
> Basically what my program does is to create a new embedded graph database,
> initalize a factory node, then it creates two threads where each of them is
> tasked with creating two nodes, giving them an ID (according to the pattern
> shown in the design guide), and that's about it.
>
> Once I run it, one thread end up on WAIT state, while the other is on
> MONITOR. Showing the threads dump, I can see that the MONITOR-state thread
> is waiting for the other thread to release the ID generation method's lock
> ("_lock"), and the WAIT-state thread is waiting inside Neo4J's RWLock
> class,
> trying to acquire a write lock, but for some reason it's waiting (line
> 326).
> It got there from a call in the ID generation method which tries to update
> the factory node's ID sequence property, as shown in the design guide.
>
>
>
> I'm appending the code here. I'm aware that this program misses some things
> (such as shutting down the DB once i'm done, adding the relationship
> between
> factory node and created node, and for the createNode() method to actually
> return a node) - i've left out everything I could to have the least amount
> of code which makes this problem occur.
>
>
> The code:
>
> public class MultiThreadingBasicTest
> {
>    private final GraphDatabaseService _graphDB;
>    private final Object _lock;
>    private final Node _factoryNode;
>
>
>    public static void main(String[] args)
>    {
>        MultiThreadingBasicTest test = new MultiThreadingBasicTest();
>        test.run();
>    }
>
>
>    public MultiThreadingBasicTest()
>    {
>        _graphDB = new EmbeddedGraphDatabase("/MTBT-graphDB");
>        _lock = new Object();
>
>        _factoryNode = createFactoryNode();
>    }
>
>
>    public void run()
>    {
>        Thread[] threads = new Thread[2];
>        for (int i=0; i < threads.length; i++)
>        {
>            threads[i] = new Thread(new Runnable()
>            {
>                @Override
>                public void run()
>                {
>                    Transaction tx = _graphDB.beginTx();
>                    try
>                    {
>                        for (int i=0; i < 2; i++)
>                        {
>                            createNode();
>                            System.out.println("echo from thread " +
> Thread.currentThread().getName());
>                        }
>
>                        tx.success();
>                    }
>                    finally
>                    {
>                        tx.finish();
>                    }
>                }
>            });
>        }
>
>
>        System.out.println("running the threads now");
>        for (Thread thread : threads)
>        {
>            thread.start();
>        }
>    }
>
>
>    private Node createFactoryNode()
>    {
>        Node factoryNode = null;
>        Transaction tx = _graphDB.beginTx();
>        try
>        {
>            factoryNode = _graphDB.createNode();
>            tx.success();
>        }
>        finally
>        {
>            tx.finish();
>        }
>        return factoryNode;
>    }
>
>
>
>    private void createNode()
>    {
>        long id = generateId();
>
> //        Node node = _graphDB.createNode();
> //
> //        node.setProperty("idseq", id);
>    }
>
>
>
>    private long generateId()
>    {
>        synchronized (_lock)
>        {
>            Long id;
>
>            if (_factoryNode.hasProperty("idseq"))
>            {
>                id = (Long) _factoryNode.getProperty("idseq");
>            }
>            else
>            {
>                id = 1L;
>            }
>            _factoryNode.setProperty("idseq", id+1);
>
>            return id;
>        }
>    }
> }
>
>
>
> (One last thing - I kept the line:
> System.out.println("echo from thread " + Thread.currentThread().getName());
> since if I remove it, the deadlock actually does not occur - at least not
> on
> my machine. However this is just a matter of race conditions, and if you
> use
> a higher number of threads / created-nodes-per-thread, then the problem
> will
> occur even without this line. I decided to keep it since I figured a case
> with two threads each creating two nodes is as simply as this can get).
>
>
> Thank you for your help,
> Ran.
>
> --
> View this message in context:
> http://neo4j-community-discussions.438527.n3.nabble.com/Node-Id-generation-deadlock-tp3473118p3473118.html
> Sent from the Neo4j Community Discussions mailing list archive at
> Nabble.com.
> _______________________________________________
> Neo4j mailing list
> [email protected]
> https://lists.neo4j.org/mailman/listinfo/user
>



-- 
David Montag <[email protected]>
Neo Technology, www.neotechnology.com
Cell: 650.556.4411
Skype: ddmontag
_______________________________________________
Neo4j mailing list
[email protected]
https://lists.neo4j.org/mailman/listinfo/user

Reply via email to