-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Handle LocalDatabase restart breaking bolt connections #9114
Conversation
b58897b
to
2444efe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mneedham changes look good to me. Made two minor comments. PR build failed with checkstyle violation: TransactionIdTrackerTest.java:67: Line has trailing spaces
.
I'll look at failing driver tests now.
@@ -64,7 +65,7 @@ | |||
TransactionStateMachineSPI( GraphDatabaseAPI db, | |||
ThreadToStatementContextBridge txBridge, | |||
QueryExecutionEngine queryExecutionEngine, | |||
TransactionIdStore transactionIdStoreSupplier, | |||
Supplier<TransactionIdStore> transactionIdStoreSupplier, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we maybe remove this param and take transactionIdStoreSupplier
out of GraphDatabaseAPI db
?
This is not very important but will reduce amount of constructor arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have changed this to get TransactionIdStore
from db.getDependencyResolver()...
@@ -109,6 +110,6 @@ private boolean isReady( long oldestAcceptableTxId ) throws TransactionFailureEx | |||
*/ | |||
public long newestEncounteredTxId() | |||
{ | |||
return transactionIdStore.getLastClosedTransactionId(); | |||
return transactionIdStoreSupplier.get().getLastClosedTransactionId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe extract transactionIdStoreSupplier.get()
in a method with a comment saying why is it required.
e0b3c4d
to
4602b55
Compare
@lutovich I've merged your branch into this one and then squashed everything. checkstyle locally seems happy so let's see if the build server agrees |
…tionIdStore not being refreshed * Introduce CANCELLED state so that polling process doesn't get restarted unless we explicitly want to restart it * Pass around a supplier of TransactionIdStore and resolve it in TransactionIdTracker * Bumped drivers to 1.2
4602b55
to
f8d8157
Compare
This happens because TransactionIdStore doesn't get refreshed after a store copy.