Skip to content

Unify Connection Acquisition Timeout with other Drivers #1292

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

Open
wants to merge 14 commits into
base: 6.x
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
this._loadBalancingStrategy = new LeastConnectedLoadBalancingStrategy(
this._connectionPool
)
this._startTime = 0
this._hostNameResolver = hostNameResolver
this._dnsResolver = new HostNameResolver()
this._log = log
Expand Down Expand Up @@ -152,6 +153,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
(error, address, conn) => this._handleSecurityError(error, address, conn, context.database)
)

this._startTime = new Date().getTime()
let conn
if (this.SSREnabled() && homeDb !== undefined && database === '') {
const currentRoutingTable = this._routingTableRegistry.get(
Expand Down Expand Up @@ -205,7 +207,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
}

try {
const connection = await this._connectionPool.acquire({ auth }, address)
const connection = await this._connectionPool.acquire({ auth, startTime: this._startTime }, address)

if (auth) {
await this._verifyStickyConnection({
Expand Down Expand Up @@ -585,7 +587,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider

async _createSessionForRediscovery (routerAddress, bookmarks, impersonatedUser, auth) {
try {
const connection = await this._connectionPool.acquire({ auth }, routerAddress)
const connection = await this._connectionPool.acquire({ auth, startTime: this._startTime }, routerAddress)

if (auth) {
await this._verifyStickyConnection({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3494,7 +3494,7 @@ describe.each([
.catch(functional.identity)

expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
expect(poolAcquire).toHaveBeenCalledWith({ auth }, server3)
expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server3)
expect(connection.release).toHaveBeenCalled()
expect(connection._sticky).toEqual(isStickyConn)
})
Expand Down Expand Up @@ -3536,7 +3536,7 @@ describe.each([
.catch(functional.identity)

expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
expect(poolAcquire).toHaveBeenCalledWith({ auth }, server1)
expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server1)
expect(connection.release).toHaveBeenCalled()
expect(connection._sticky).toEqual(isStickyConn)
})
Expand Down Expand Up @@ -3580,7 +3580,7 @@ describe.each([
.catch(functional.identity)

expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
expect(poolAcquire).toHaveBeenCalledWith({ auth }, server0)
expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server0)
expect(connection.release).toHaveBeenCalled()
expect(connection._sticky).toEqual(isStickyConn)
})
Expand Down Expand Up @@ -3609,7 +3609,7 @@ describe.each([
.catch(functional.identity)

expect(error).toEqual(newError('Driver is connected to a database that does not support user switch.'))
expect(poolAcquire).toHaveBeenCalledWith({ auth }, server0)
expect(poolAcquire).toHaveBeenCalledWith({ auth, startTime: expect.any(Number) }, server0)
expect(connection.release).toHaveBeenCalled()
expect(connection._sticky).toEqual(isStickyConn)
})
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/internal/pool/pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type ValidateOnRelease<R extends unknown = unknown> = (resource: R) => (Promise<
type InstallObserver<R extends unknown = unknown> = (resource: R, observer: unknown) => void
type RemoveObserver<R extends unknown = unknown> = (resource: R) => void
interface AcquisitionConfig { requireNew?: boolean }
interface AcquisitionContext { startTime?: number, forceReAuth?: boolean, skipReAuth?: boolean, auth?: object, triggerValidation?: boolean }

interface ConstructorParam<R extends unknown = unknown> {
create?: Create<R>
Expand Down Expand Up @@ -111,12 +112,13 @@ class Pool<R extends unknown = unknown> {
* @param {boolean} config.requireNew Indicate it requires a new resource
* @return {Promise<Object>} resource that is ready to use.
*/
async acquire (acquisitionContext: unknown, address: ServerAddress, config?: AcquisitionConfig): Promise<R> {
async acquire (acquisitionContext: AcquisitionContext, address: ServerAddress, config?: AcquisitionConfig): Promise<R> {
const key = address.asKey()

// We're out of resources and will try to acquire later on when an existing resource is released.
const allRequests = this._acquireRequests
const requests = allRequests[key]
const elapsedTime = (acquisitionContext.startTime != null && acquisitionContext.startTime !== 0) ? new Date().getTime() - acquisitionContext.startTime : 0
if (requests == null) {
allRequests[key] = []
}
Expand All @@ -143,7 +145,7 @@ class Pool<R extends unknown = unknown> {
)
)
}
}, this._acquisitionTimeout)
}, this._acquisitionTimeout - elapsedTime)

if (typeof timeoutId === 'object') {
// eslint-disable-next-line
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions packages/neo4j-driver-deno/lib/core/internal/pool/pool.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions packages/testkit-backend/src/skipped-tests/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,6 @@ const skippedTests = [
skip(
'Needs trying all DNS resolved addresses for hosts in the routing table',
ifEndsWith('.test_ipv6_read').and(startsWith('stub.routing.test_routing_'))
),
skip(
'Driver has separate timeouts for every connection it attempts to open. This will change in 6.0',
ifEquals('stub.homedb.test_homedb.TestHomeDbMixedCluster.test_connection_acquisition_timeout_during_fallback')
)
]

Expand Down
2 changes: 1 addition & 1 deletion testkit/testkit.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"testkit": {
"uri": "https://github.com/neo4j-drivers/testkit.git",
"ref": "6.x"
"ref": "connection-acquisition-6x-change"
}
}