-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expired transactions #145
Comments
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? |
Well, I expect that by calling |
But if your transaction has expired, shouldn't it give you an error so you can react to it? We could definitely add an |
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. |
So once we get a response saying that a transaction is expired, it should mark it, But are you saying that it should automatically create a new transaction so that |
No, I think 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. |
Maybe I'm wrong and neo4j-core should not be responsible for removing expired transactions. |
I don't really know... I think the question is whether |
But I do think that we should maybe subclass |
That sounds good. |
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. |
Here is it. #156 |
Transaction
does not update expiration timestamp and does not unregister expired transaction.The text was updated successfully, but these errors were encountered: