Expired transactions

[dpisarewski]

Transaction does not update expiration timestamp and does not unregister expired transaction.

>> tx = ::Neo4j::Transaction.new
#<Neo4j::Server::CypherTransaction:0x007fcb311a0280 @connection=#<Faraday::Connection:0x007fcb29d4a9f8 @parallel_manager=nil, @headers={"Content-Type"=>"application/json", "User-Agent"=>"neo4j gem/3.0.4 (https://github.com/neo4jrb/neo4j)"}, @params={}, @options=#<Faraday::RequestOptions (empty)>, @ssl=#<Faraday::SSLOptions (empty)>, @default_parallel_manager=nil, @builder=#<Faraday::RackBuilder:0x007fcb29d4a458 @handlers=[FaradayMiddleware::EncodeJson, FaradayMiddleware::ParseJson, Faraday::Adapter::NetHttpPersistent], @app=#<FaradayMiddleware::EncodeJson:0x007fcb2a717cb0 @app=#<FaradayMiddleware::ParseJson:0x007fcb2a717d78 @app=#<Faraday::Adapter::NetHttpPersistent:0x007fcb2a717dc8 @app=#<Proc:0x007fcb2a717f58@/Users/dieter/.rvm/gems/ruby-2.1.2@8actions/gems/faraday-0.9.0/lib/faraday/rack_builder.rb:152 (lambda)>>, @options={:content_type=>"application/json"}, @content_types=["application/json"]>>>, @url_prefix=#<URI::HTTP:0x007fcb29d4a048 URL:http:/>, @proxy=nil>, @commit_url="http://localhost:7474/db/data/transaction/6/commit", @exec_url="http://localhost:7474/db/data/transaction/6", @resource_url="http://localhost:7474/db/data/transaction", @resource_data={"commit"=>"http://localhost:7474/db/data/transaction/6/commit", "results"=>[], "transaction"=>{"expires"=>"Tue, 09 Dec 2014 12:38:34 +0000"}, "errors"=>[]}, @pushed_nested=0>

>> Thread.current[:neo4j_curr_tx].resource_data
{"commit"=>"http://localhost:7474/db/data/transaction/6/commit", "results"=>[], "transaction"=>{"expires"=>"Tue, 09 Dec 2014 12:38:34 +0000"}, "errors"=>[]}

>> User
User(access_token: Object, created_at: DateTime, email: Object, id: Object, password_digest: Object, updated_at: DateTime, username: Object)

>> sleep 60
60

>> User.first
 CYPHER 9ms MATCH (n:`User`) RETURN n ORDER BY n.uuid LIMIT 1
Neo4j::Session::CypherError: Unrecognized transaction id. Transaction may have timed out and been rolled back.
    from /Users/dieter/.rvm/gems/ruby-2.1.2@8actions/gems/neo4j-core-3.0.8/lib/neo4j-server/cypher_response.rb:168:in `raise_cypher_error'
    from /Users/dieter/.rvm/gems/ruby-2.1.2@8actions/gems/neo4j-core-3.0.8/lib/neo4j-core/query.rb:163:in `response'
    from /Users/dieter/.rvm/gems/ruby-2.1.2@8actions/gems/neo4j-core-3.0.8/lib/neo4j-core/query.rb:204:in `pluck'
    from /Users/dieter/.rvm/gems/ruby-2.1.2@8actions/bundler/gems/neo4j-0e776e8ece07/lib/neo4j/active_node/query_methods.rb:15:in `first'
    from (irb):9
>> Thread.current[:neo4j_curr_tx].resource_data
{"commit"=>"http://localhost:7474/db/data/transaction/6/commit", "results"=>[], "transaction"=>{"expires"=>"Tue, 09 Dec 2014 12:38:34 +0000"}, "errors"=>[]}

[subvertallchris]

What do you think the default behavior should be? I guess it should mark the transaction as expired once this error occurs so it doesn't bother attempting to continue, right?

[dpisarewski]

Well, I expect that by calling Neo4j::Transaction.current I won't get an expired transaction. It could also update the value of 'expires' since neo4j rest api always returns it. http://neo4j.com/docs/stable/rest-api-transactional.html#rest-api-execute-statements-in-an-open-transaction-in-rest-format-for-the-return

[subvertallchris]

But if your transaction has expired, shouldn't it give you an error so you can react to it? We could definitely add an expired? instance method that compares the current time to the transaction's expiration, too.

[dpisarewski]

I agree that all requests inside an invalid transaction must fail. But when you already know the transaction is invalid then you don't need to send requests to the database. They could fail without touching the database due to expired transaction. Furthermore, in this case we also know the transaction is already closed on the server. So after throwing an exception it Neo4j::Transaction.current should be available for registering a new transaction without calling close method on the expired transaction.

[subvertallchris]

So once we get a response saying that a transaction is expired, it should mark it, expired? should return true, and all attempts to communicate with the server using that transaction object should fail. If we already know that the transaction is invalid, we shouldn't be wasting time trying to reuse it -- I'm with you there. I don't want to examine the expiration timestamp on each action, though... This should be reactive, not proactive.

But are you saying that it should automatically create a new transaction so that Neo4j::Transaction.current returns a valid object?

[dpisarewski]

No, I think Neo4j::Transaction.current should be nil as there is no valid transaction. And I should be able to create a new transaction and get this transaction by calling Neo4j::Transaction.current.

Example for firing request on invalid transaction:

def test
  tx = Neo4j::Transaction.new
  sleep 60 #long running operation
  User.create
rescue
  tx.failure
ensure
  tx.close    #bang! Neo4j::Server::Resource::ServerException
end

test

Example for unregistered expired transaction:

#first context(code can't be changed)
def call
  SecondContext.start
  AnotherContext.make_changes   #bang!
  SecondContext.finalize       #won't execute
end

#second context
def start
  @tx = Neo4j::Transaction.new
rescue
  @tx.failure
end

def finalize
  @tx.close rescue nil
end

#another context
def make_changes
  sleep 60   #long running operation
  SomeNode.create    #bang!
end

According to the second example, the invalid transaction won't be closed and the next call will also fail.

[dpisarewski]

Maybe I'm wrong and neo4j-core should not be responsible for removing expired transactions.

[subvertallchris]

I don't really know... I think the question is whether current represents a transaction with the server or a transaction object within the app. As far as I'm concerned, it's an object within the app, so user owns it and it's up to them to be aware of its state. Once a transaction is failed or expired, the state of the object changes, but their current transaction object still exists.

[subvertallchris]

But I do think that we should maybe subclass CypherTransaction, maybe FailedCypherTransaction or both that and ExpiredCypherTransaction, automatically created and assigned to current when the state of a transaction changes. It'll give more control over behavior and make it easier for users to identify and react to error states.

[dpisarewski]

That sounds good.

[subvertallchris]

I'm not going to add a new class for failed transactions right now, a little too much going on. I was going to modify the behavior to prevent continued queries within a transaction that's already been marked failed but just noticed this:

      it 'can continue operations after transaction is rolled back' do
        node = Neo4j::Node.create(name: 'andreas')
        Neo4j::Transaction.run do |tx|
          tx.failure
          node[:name] = 'foo'
          expect(node[:name]).to eq('foo')
        end
        expect(node['name']).to eq('andreas')
      end

Interesting that it's deliberate.

Since we know that you can't do anything at all with an expired transaction, as soon as an error comes back stating that the transaction is expired, it will be marked and subsequent queries will not run.

We can play with a new class for failed transactions if you want. I think the next release is gonna be 4.0 so now is a good opportunity to change the API.

[dpisarewski]

Here is it. https://github.com/neo4jrb/neo4j-core/pull/156