Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-765

Decompose AbstractTransaction for different transactional contexts

    Details

      Description

      EventStrategy uses the listeners available on Transaction implementations to know when events should be fired (i.e. per transaction). Validate that there are no listener leaks in this model and consider methods for improvement if possible.

        Activity

        Hide
        spmallette stephen mallette added a comment -

        There's currently no way to know when the user is done with the EventStrategy, so knowing when to remove the Transaction listener is not really possible. I created TINKERPOP3-790 to add AutoCloseable to TraversalSource which will give us that hook to do the removal. Bumped this change to 3.1.0 since it required an API change.

        Show
        spmallette stephen mallette added a comment - There's currently no way to know when the user is done with the EventStrategy , so knowing when to remove the Transaction listener is not really possible. I created TINKERPOP3-790 to add AutoCloseable to TraversalSource which will give us that hook to do the removal. Bumped this change to 3.1.0 since it required an API change.
        Hide
        spmallette stephen mallette added a comment -

        I think AbstractTransaction is bound too tightly to the ThreadLocal concept. That doesn't work too well for graphs implementing threaded transaction and that then want to use that class. I think the better approach is to remove the implementation of the "listener" methods from AbstractTransaction and re-implement them in two sub-classes: one for ThreadLocal implementations as it is now and one that assumes concurrent access. Implementers can then choose which is right for them to base their implementation on. Another reason to bump this to 3.1.0 given some API change.

        Show
        spmallette stephen mallette added a comment - I think AbstractTransaction is bound too tightly to the ThreadLocal concept. That doesn't work too well for graphs implementing threaded transaction and that then want to use that class. I think the better approach is to remove the implementation of the "listener" methods from AbstractTransaction and re-implement them in two sub-classes: one for ThreadLocal implementations as it is now and one that assumes concurrent access. Implementers can then choose which is right for them to base their implementation on. Another reason to bump this to 3.1.0 given some API change.
        Hide
        spmallette stephen mallette added a comment -

        In 3.0.0, implementers typically used AbstractTransaction as a starting point for their Transaction implementation. As a result of this change, they will need to extend from AbstractThreadLocalTransaction to get the same functionality as before. Not making this change will likely manifest itself as compile errors in their Transaction implementations.

        There is also a AbstractThreadedTransaction which takes a global approach to raising events on commit. This would be useful for those graphs implementing threaded transactions (i.e. for the Graph returned from Transaction.newThreadedTx()..

        Show
        spmallette stephen mallette added a comment - In 3.0.0, implementers typically used AbstractTransaction as a starting point for their Transaction implementation. As a result of this change, they will need to extend from AbstractThreadLocalTransaction to get the same functionality as before. Not making this change will likely manifest itself as compile errors in their Transaction implementations. There is also a AbstractThreadedTransaction which takes a global approach to raising events on commit. This would be useful for those graphs implementing threaded transactions (i.e. for the Graph returned from Transaction.newThreadedTx() ..

          People

          • Assignee:
            spmallette stephen mallette
            Reporter:
            spmallette stephen mallette
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development