Fixed Bug:- Multi-Driver Failover when one of multiple hosts down#282
Conversation
|
|
||
| $client = new Client($driverSetupManager, $sessionConfig, $transactionConfig); | ||
|
|
||
| $this->expectException(RuntimeException::class); |
There was a problem hiding this comment.
@p123-stack, I am missing the test where we are testing what happens during a connection exception when statements are being run.
There was a problem hiding this comment.
We added tests where runStatements throws ConnectionPoolException (retry + resetDriver). Say if you also want one with ConnectException explicitly.
…failure during statement execution
transistive
left a comment
transistive
left a comment
There was a problem hiding this comment.
Almost there! Good stuff. We can probably simplify some of the tests to get the same amount of coverage, and we can have a simple solution by adding reset logic to the driver setup manager
| $successfulSessionMock->method('runStatements') | ||
| ->willReturn($expectedResult); | ||
|
|
||
| $driverSetupManager->expects($this->exactly(2)) |
There was a problem hiding this comment.
I can see here that this won't work perfectly. You are mocking the driver setup manager to return different drivers on subsequent calls, that is not how the driver setup manager operates. It will always return the same driver
There was a problem hiding this comment.
Agreed — we implemented resetDriver on DriverSetupManager and call it from executeWithRetry before retry, so getDriver isn’t expected to “rotate” by itself. Tests now include one that asserts resetDriver + retry.
|
Looks like this should fix #281 - thanks! |
| { | ||
| public function testClientRunStatementWithFailingDriver(): void | ||
| { | ||
| $driverSetupManager = $this->createMock(DriverSetupManager::class); |
There was a problem hiding this comment.
We'll have to improve testing here to see if the recovery works
There was a problem hiding this comment.
Added unit tests that exercise the retry path: after a ConnectionPoolException, resetDriver is called and the next getDriver succeeds for writeTransaction, readTransaction, and beginTransaction. run / runStatements retry behavior was already covered in ClientSessionExceptionHandlingTest. Resolving—let me know if you want an integration-style test as well.
…in verifyConnectivity and test client recovery
…tion/routing failures
Resolve RoutingTable: combine hasServer (in_array) with removeServer returning self when unchanged. Made-with: Cursor
3 participants
Multi-Driver Failover When Primary Node Is Down
Failover Handling Implementation
Added support for failover when the highest-priority Neo4j driver is unavailable.
DriverSetupManager now continues to next available driver instead of failing immediately.
Exception Handling Improvements
Neo4jConnectionPool::acquire() now throws ConnectionPoolException instead of RuntimeException.
Neo4jDriver::verifyConnectivity() catches both ConnectException and RuntimeException.
Defensive try-catch added in DriverSetupManager::getDriver() to log and skip failed drivers.
Routing & Connectivity
Client properly attempts secondary drivers when primary fails.
Connection pool and driver retry logic updated to ensure uninterrupted operations.
Testing & Compliance
Unit tests added in MultiDriverFailoverTest and ClientExceptionHandlingTest.
Tests verify driver fallback, client-level transactions, and complete failover flow.