Uploaded image for project: 'Blazegraph (by SYSTAP)'
  1. Blazegraph (by SYSTAP)
  2. BLZG-2041

BigdataSail should not locate the AbstractTripleStore until a connection is requested

    Details

      Description

      See BLZG-2023. One of the underlying issues is that we are locating the unisolated view of the AbstractTripleStore in the BigdataSail constructor rather than waiting until we obtain an unisolated connection. This forces some of the lock handling logic onto the caller and is the root cause of BLZG-2023. We do have a fix for BLZG-2023, but it pushes more responsibility onto the caller. This ticket is to explore an alternative which encapsulates that complexity in BigdataSail by deferring the resolution of the AbstractTripleStore.

      We are exploring pushing down the locate of the unisolated view of the AbstractTripleStore into BigdataSail.getXXXConnection(). For the unisolated connection (getUnisolatedConnection()), this would defer the locate until we have the semaphore and clean up the external lock coordination. Doing this would mean removing the following fields and access methods from BigdataSail and BigdataSailRepository, pushing them down onto BigdataSailConnection and BigdataSailRepositoryConnection.

      Remove:

      • BigdataSail.AbstractTripleStore:database, getDatabase() / getAbstractTripleStore()
      • BigdataSail.getInferenceEngine() (this one is easy since it is only used by BigdataSailConnection anyway).

      Add:

      • BigdataSail.getIndexManager()
      • BigdataSail.getNamespace()
      • BigdataSail.indexManager:IIndexManager
      • BigdataSail.namespace:String (the namespace name)

      Problems:

      • BigdataSail.getValueFactory() defers to the LexiconRelation constructor, which allows override of the ValueFactory implementation class. Could probably be replaced by just "valueFactory = BigdataValueFactoryImpl.getInstance(namespace);" which could be directly implemented by BigdataSail.
      • BigdataSail.getWritable() - this would need to be changed to indicate whether or not the BigdataSail was willing to allow the caller to obtain a writable connection. I think that we always allow this, so it might just return "true".

      We are working through the implications of these changes. BigdataSail.getDatabase() is used quite broadly. As long as we have a BigdataSailConnection object, we can simply access the AbstractTripleStore on that connection. However, if we have only the BigdataSail then we might need to modify the caller to explicitly obtain, use, and release a BigdataSailConnection for the operation. In most circumstances, the unisolated connection or a read-only connection on the lastCommitTime would be the most appropriate connection objects to obtain.

      Note: One possible consequence of this ticket would be slightly more overhead when obtaining a connection from the BigdataSail.

      Note: This change set would likely conflict with BLZG-420. We might have to redo BLZG-420 against the current master, which might be a good idea anyway.

      Brad Bebee
      martyncutcher
      michaelschmidt

      1. screenshot-1.png
        241 kB

        Issue Links

          Activity

          Hide
          michaelschmidt michaelschmidt added a comment -

          Here's a summary of the benchmark results:

          Show
          michaelschmidt michaelschmidt added a comment - Here's a summary of the benchmark results: SP2Bench: https://docs.google.com/spreadsheets/d/1xtOf9C-SycmynC8tZg6aDkjVWU0dcsj-l_da4o0gjtw/edit#gid=1996827133 => stable Govtrack: https://docs.google.com/spreadsheets/d/1MFF3kQmzQv7LzBFjbNX7bhc1eLgCVPQH0vQA6SxeHuQ/edit#gid=1823120024 => stable LUBM: https://docs.google.com/spreadsheets/d/12rbe77GOqnRmi4yFjWE1D1hXnd2jB-WPUV2uVJZEmwE/edit#gid=2003180795 => stable BSBM: https://docs.google.com/spreadsheets/d/1i-JnEy_W5Pt4AWg87oxg564GYkz3zaxxmIS9H4OKssE/edit#gid=936547171 => results for the bsbm explore look good (even a bit faster than the release, but I would say that might be a statistical deviation). The BSBM Explore+Update all failed. I'm attaching the sparql-server.log file, it shows plenty of errors (I did not yet verify they come from Explore+Update, but I guess it's reasonable to assume so).
          Hide
          michaelschmidt michaelschmidt added a comment -

          Server logs for BSBM benchmark (showing a variety of failures)

          Show
          michaelschmidt michaelschmidt added a comment - Server logs for BSBM benchmark (showing a variety of failures)
          Hide
          bryanthompson bryanthompson added a comment -

          martyncutcher A problem remains when the shutdown of the sail is being invoked from a thread which did not obtain the connection. In this case, an exception is thrown and the shutdown fails.

          ERROR: 340413      main com.bigdata.rdf.sail.BigdataSail.__tearDownUnitTest(BigdataSail.java:1059): Problem during shutdown: java.lang.IllegalMonitorStateException
          java.lang.IllegalMonitorStateException
          	at java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryRelease(ReentrantReadWriteLock.java:374)
          	at java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1260)
          	at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.unlock(ReentrantReadWriteLock.java:1131)
          	at com.bigdata.rdf.sail.BigdataSail$BigdataSailConnection.close(BigdataSail.java:4047)
          	at com.bigdata.rdf.sail.SailBase.shutDown(SailBase.java:216)
          
          Show
          bryanthompson bryanthompson added a comment - martyncutcher A problem remains when the shutdown of the sail is being invoked from a thread which did not obtain the connection. In this case, an exception is thrown and the shutdown fails. ERROR: 340413 main com.bigdata.rdf.sail.BigdataSail.__tearDownUnitTest(BigdataSail.java:1059): Problem during shutdown: java.lang.IllegalMonitorStateException java.lang.IllegalMonitorStateException at java.util.concurrent.locks.ReentrantReadWriteLock$Sync.tryRelease(ReentrantReadWriteLock.java:374) at java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1260) at java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.unlock(ReentrantReadWriteLock.java:1131) at com.bigdata.rdf.sail.BigdataSail$BigdataSailConnection.close(BigdataSail.java:4047) at com.bigdata.rdf.sail.SailBase.shutDown(SailBase.java:216)
          Hide
          michaelschmidt michaelschmidt added a comment -

          Here's a summary of the benchmark results for the BLZG-2041-dbg branch (bbfa3b703b628b33a2b22a766ce8a13e4bfd177b), replicated from the PR discussion:

          Show
          michaelschmidt michaelschmidt added a comment - Here's a summary of the benchmark results for the BLZG-2041 -dbg branch (bbfa3b703b628b33a2b22a766ce8a13e4bfd177b), replicated from the PR discussion: SP2Bench: https://docs.google.com/spreadsheets/d/1xtOf9C-SycmynC8tZg6aDkjVWU0dcsj-l_da4o0gjtw/edit#gid=928552531 => stable LUBM: https://docs.google.com/spreadsheets/d/12rbe77GOqnRmi4yFjWE1D1hXnd2jB-WPUV2uVJZEmwE/edit#gid=1493517174 => results OK, about 9% speedup (this is about the same as we observed for the latest 2.2 RC) BSBM: https://docs.google.com/spreadsheets/d/1i-JnEy_W5Pt4AWg87oxg564GYkz3zaxxmIS9H4OKssE/edit#gid=511311210 => results good, very much similiar to previously observed results (no errors, speedups of ~5% for the single threaded runs, significant speedups of 25-30% for the multi-threaded explore+update runs) Govtrack: https://docs.google.com/spreadsheets/d/1MFF3kQmzQv7LzBFjbNX7bhc1eLgCVPQH0vQA6SxeHuQ/edit#gid=2032957091 => about -30% performance (same as previous BLZG-533 benchmarks) + query10 yielding errors: " [java] ERROR: Haltable.java:469: com.bigdata.util.concurrent.Haltable@42d669c : isFirstCause=true : java.lang.Exception: task=ChunkTask {query=9c2b393d-8074-4511-9b38-e88a08601d98,bopId=51,partitionId=-1,sinkId=54,altSinkId=null} , cause=java.util.concurrent.ExecutionException: java.lang.RuntimeException: java.lang.IllegalStateException: Check blob header assumptions" (see PR comment at https://github.com/SYSTAP/bigdata/pull/475#issuecomment-241267010 for log file)
          Hide
          martyncutcher martyncutcher added a comment -

          I have pushed a fix to an "off by one" boundary calculation on BLOB data for the MemoryManager, and added a testBlobStreamBoundaries test to TestMemoryManager.

          Show
          martyncutcher martyncutcher added a comment - I have pushed a fix to an "off by one" boundary calculation on BLOB data for the MemoryManager, and added a testBlobStreamBoundaries test to TestMemoryManager.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: