Details

    • Type: Sub-task
    • Status: Done
    • Priority: Highest
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: BLAZEGRAPH_2_2_0
    • Component/s: Bigdata SAIL
    • Labels:
      None

      Description

      The BigdataSail.getConnection() code path does correctly register new connections with the SailBase and hence BigdataSail.shutdown() does close such connections (after a 20 second timeout). However, code which directly uses the following methods does not cause the connection to be registered in SailBase.activeConnections and hence such connections are not automatically closed.

      • getReadOnlyConnection(...)
      • getUnisolatedConnection()
      • getReadWriteConnection()
        With BLZG-2041, this is leading to a deadlock in the tinkerpop test suite due to a related bug fix which made BigdataSail.lock canonical across all open instances of a BigdataSail for the same namespace.

        Activity

        Hide
        bryanthompson bryanthompson added a comment -

        Issue was diagnosed as a long-standing problem. The SailBase.activeConnections map in the parent class was not being maintained along all code paths which could return a BigdataSailConnection. Resolved by martyncutcher and in CI now.

        mikepersonick Note that a *20 second* pause can now be observed if a unit test fails to close a connection and then calls BigdataSail.shutdown(). It will time out the connection and then close it, but this is not immediate.

        Brad Bebee

        Show
        bryanthompson bryanthompson added a comment - Issue was diagnosed as a long-standing problem. The SailBase.activeConnections map in the parent class was not being maintained along all code paths which could return a BigdataSailConnection. Resolved by martyncutcher and in CI now. mikepersonick Note that a * 20 second * pause can now be observed if a unit test fails to close a connection and then calls BigdataSail.shutdown(). It will time out the connection and then close it, but this is not immediate. Brad Bebee
        Hide
        bryanthompson bryanthompson added a comment -

        Hung build. The root cause is that BigdataSail.shutdown() now gets into trouble because the active connections own a lock and another thread can not close them. Martyn is having a think.

        See https://ci.blazegraph.com/job/bigdata-github-maven-PR-tester/1064/

        We need some new tests which examine:

        • Shutdown of a BigdataSail from a thread which did not acquire a connection that is still open at the time of the shutdown.
        • Releasing a connection from another thread.
        • Contention should exist between two BigdataSail views of the same namespace. This can either be done by sharing some synchronization (the BigdataSail.lock) or by delegating the BigdataSail implementation to a backing object which is canonical per namespace.

        martyncutcher

        Show
        bryanthompson bryanthompson added a comment - Hung build. The root cause is that BigdataSail.shutdown() now gets into trouble because the active connections own a lock and another thread can not close them. Martyn is having a think. See https://ci.blazegraph.com/job/bigdata-github-maven-PR-tester/1064/ We need some new tests which examine: Shutdown of a BigdataSail from a thread which did not acquire a connection that is still open at the time of the shutdown. Releasing a connection from another thread. Contention should exist between two BigdataSail views of the same namespace. This can either be done by sharing some synchronization (the BigdataSail.lock) or by delegating the BigdataSail implementation to a backing object which is canonical per namespace. martyncutcher
        Hide
        bryanthompson bryanthompson added a comment -

        Here is a proposed approach:

        • SailBase.activeConnections tracks the Thread for each requester (or a Pair(Thread,StackTrace)).
        • shutdown() sets an internal runState:=shutdown
        • shutdown() interrupts the Thread for each activeConnection. Those threads should pop up pretty much immediately to BigdataSailConnection.close() if the application logic is well written.
          • Note: activeConnection MUST be careful to not hold the Thread reference beyond the management life cycle of the connection since we do not want to interrupt a thread which is now doing something else!
          • Note: BigdataSailConnection implements finalize()
        • Once the 20 second timeout expires with at least once activeConnection, BigdataSail.shutdown():
          • Elevates the BigdataSail.runState:=shutdownNow
          • shoots the BigdataSail.lock in the canoncializing map. This means that any subsequent BigdataSail wrapping the namespace will have a different lock contending between read/write and unisolated operations for that namespace.
          • BigdataSailConnection.close() DOES NOT RELEASE THE LOCK if BigdataSail.runState==shutdownNow and the lock is not held by the current thread. This avoids an IllegalMonitorStateException which would otherwise be thrown since the thread calling BigdataSail.shutdown() can not release a lock associated with another thread.
        Show
        bryanthompson bryanthompson added a comment - Here is a proposed approach: SailBase.activeConnections tracks the Thread for each requester (or a Pair(Thread,StackTrace)). shutdown() sets an internal runState:=shutdown shutdown() interrupts the Thread for each activeConnection. Those threads should pop up pretty much immediately to BigdataSailConnection.close() if the application logic is well written. Note: activeConnection MUST be careful to not hold the Thread reference beyond the management life cycle of the connection since we do not want to interrupt a thread which is now doing something else! Note: BigdataSailConnection implements finalize() Once the 20 second timeout expires with at least once activeConnection, BigdataSail.shutdown(): Elevates the BigdataSail.runState:=shutdownNow shoots the BigdataSail.lock in the canoncializing map. This means that any subsequent BigdataSail wrapping the namespace will have a different lock contending between read/write and unisolated operations for that namespace. BigdataSailConnection.close() DOES NOT RELEASE THE LOCK if BigdataSail.runState==shutdownNow and the lock is not held by the current thread. This avoids an IllegalMonitorStateException which would otherwise be thrown since the thread calling BigdataSail.shutdown() can not release a lock associated with another thread.
        Hide
        bryanthompson bryanthompson added a comment -

        This is not yet correct. After further discussion, we believe that we need to create a specialized object to handle the synchronization decision between allowing an unisolated connection for a specific namespace which must be contended with a read/write connection for the same namespace.

        Show
        bryanthompson bryanthompson added a comment - This is not yet correct. After further discussion, we believe that we need to create a specialized object to handle the synchronization decision between allowing an unisolated connection for a specific namespace which must be contended with a read/write connection for the same namespace.

          People

          • Assignee:
            martyncutcher martyncutcher
            Reporter:
            bryanthompson bryanthompson
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: