orientechnologies / orientdb

OrientDB is the most versatile DBMS supporting Graph, Document, Reactive, Full-Text and Geospatial models in one Multi-Model product. OrientDB can run distributed (Multi-Master), supports SQL, ACID Transactions, Full-Text indexing and Reactive Queries.
https://orientdb.dev
Apache License 2.0
4.73k stars 869 forks source link

Nested calls to ODatabaseDocumentPool.global.acquire() results in unclosed connection #1942

Closed RoarN closed 10 years ago

RoarN commented 10 years ago

Version: 1.6.1, 1.6.3

Nested use of ODatabaseDocumentPool.global.acquire() results in unclosed connection in the pool.

Test: Set pool size to 5 to make the test shorter Call method1 that acquires a connection Call method2 from method1, method2 also acquires a connection Iterate 10 times and call method1

Result The test fails after 5 iterations

2014-01-09 10:59:53,064 INFO main - method1() 2014-01-09 10:59:53,078 INFO main - method2() 2014-01-09 10:59:53,081 INFO main - method1() 2014-01-09 10:59:53,084 INFO main - method2() 2014-01-09 10:59:53,087 INFO main - method1() 2014-01-09 10:59:53,090 INFO main - method2() 2014-01-09 10:59:53,091 INFO main - method1() 2014-01-09 10:59:53,095 INFO main - method2() 2014-01-09 10:59:53,095 INFO main - method1() 2014-01-09 10:59:53,099 INFO main - method2() 2014-01-09 10:59:53,099 INFO main - method1() 2014-01-09 10:59:58,101 FATAL main - acquire failed com.orientechnologies.common.concur.lock.OLockException: Not more resources available in pool. Requested resource: remote:localhost/test at com.orientechnologies.common.concur.resource.OResourcePool.getResource(OResourcePool.java:46) at com.orientechnologies.orient.core.db.ODatabasePoolAbstract.acquire(ODatabasePoolAbstract.java:93) at com.orientechnologies.orient.core.db.ODatabasePoolAbstract.acquire(ODatabasePoolAbstract.java:78) at com.orientechnologies.orient.core.db.ODatabasePoolBase.acquire(ODatabasePoolBase.java:121) at com.ceeview.test.db.orient.pool.PoolTester.getConnection(PoolTester.java:46) at com.ceeview.test.db.orient.pool.PoolTester.method2(PoolTester.java:58) at com.ceeview.test.db.orient.pool.PoolTester.method1(PoolTester.java:75) at com.ceeview.test.db.orient.pool.PoolTester.test(PoolTester.java:87) at com.ceeview.test.db.orient.pool.PoolTester.main(PoolTester.java:94) 2014-01-09 10:59:58,106 INFO main - method2() Exception in thread "main" java.lang.NullPointerException at com.ceeview.test.db.orient.pool.PoolTester.method2(PoolTester.java:65) at com.ceeview.test.db.orient.pool.PoolTester.method1(PoolTester.java:75) at com.ceeview.test.db.orient.pool.PoolTester.test(PoolTester.java:87) at com.ceeview.test.db.orient.pool.PoolTester.main(PoolTester.java:94)

Expected: Run all iterations without any problems.

FYI. If I instead call method2 in the test it works just fine.

RoarN commented 10 years ago

public class PoolTester { static Logger logger = Logger.getLogger(PoolTester.class.getName());

ODatabaseDocumentPool pool = ODatabaseDocumentPool.global(1, 5);

public OrientGraph getConnection()
{
    String url = "remote:localhost/test";
    String user = "admin";
    String password = "admin";
    try
    {
        ODatabaseDocumentTx conn = pool.acquire(url, user, password);           
        return new OrientGraph(conn);           
    }
    catch (RuntimeException e)
    {
        logger.fatal("acquire failed",e);
    }
    return null;
}

private void method2()
{
    OrientGraph conn = getConnection();
    try
    {
        logger.info("method2()");
    }
    finally
    {
        conn.shutdown();
    }
}

private void method1()
{
    OrientGraph conn = getConnection();
    try
    {
        logger.info("method1()");
        method2();
    }
    finally
    {
        conn.shutdown();
    }
}

public void test()
{
    for (int i = 0; i < 10; i++)
    {
        method1();
    }
}

public static void main(String[] args)
{
    PoolTester test = new PoolTester();
    test.test();    
}

}

lvca commented 10 years ago

The problem here is the management of the context in thread local. When you close the connection in nested method, the thread local is reset and the outside close doesn't find the db to close.

lvca commented 10 years ago

We could fix it in different ways: 1) support stack of database thread 2) create a new method "getOrAcquire()" that re-use a previous connection if already in thread.

Solution (2) is the fastest

RoarN commented 10 years ago

Solution 1 is of course preferred, but I think I can live with solution 2 if we can get it fast.

Regarding solution 2, how will shutdown() and transactions be handled?

andrii0lomakin commented 10 years ago

Hi, I think we need to mix given solutions, we will use connection with counter of usages so shutdown and close will be handled without problems. Also I will add one more acquire method with reusePreviousConnection parameter set to true by default. Also I will change OTransaction.begin method of tx in such way that transactions will be reused by default and will add begin with two parameters reuse and rollbackIfActive so we will control tx behavior if tx is active but by default transactions will be propagated (reused).

lvca commented 10 years ago

:+1: