Add ConnectionLivenessCheckTimeout configuration (#551)

* Add  configuration

* Added new unit test and skipped Testkit test

* Mock time globally, not per struct

Pros:
 * easier to mock time functions
   (this commit adds mocking time.Since)
 * less passing time functions through the whole stack
 * better performing when mocking is not needed:
   * no dynamic dispatch needed
   * compiler might inline calls

Cons:
 * requires locking when mocking time
 * tests that need mocking time cannot run in parallel
 * more spooky action at a distance

---------

Co-authored-by: Robsdedude <rouven.bauer@neo4j.com>

README.md

@@ -368,7 +368,7 @@ modules](https://go.dev/ref/mod) for dependency resolution.
 You can run unit tests as follows:
 
 ```shell
-go test -tags=internal_testkit -short ./...
+go test -tags internal_testkit,internal_time_mock -short ./...
 ```
 
 ### Integration and Benchmark Testing

hooks/pre-commit

@@ -13,11 +13,11 @@ fi
 echo "# pre-commit hook"
 printf '%-15s' "## staticcheck "
 cd "$(mktemp -d)" && go install honnef.co/go/tools/cmd/staticcheck@"${staticcheck_version}" && cd - > /dev/null
-"${GOBIN:-$(go env GOPATH)/bin}"/staticcheck -tags internal_testkit ./...
+"${GOBIN:-$(go env GOPATH)/bin}"/staticcheck -tags internal_testkit,internal_time_mock ./...
 echo "✅"
 
 printf '%-15s' "## go vet "
-go vet -tags internal_testkit ./...
+go vet -tags internal_testkit,internal_time_mock ./...
 echo "✅"
 
 printf '%-15s' "## go test "

neo4j/auth/auth.go

@@ -19,12 +19,14 @@ package auth
 
 import (
  "context"
+ "reflect"
+ "time"
+
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/db"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/auth"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/collections"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/racing"
- "reflect"
- "time"
+ itime "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/time"
 )
 
 // TokenManager is an interface for components that can provide auth tokens.
@@ -63,7 +65,6 @@ type neo4jAuthTokenManager struct {
  token *auth.Token
  expiration *time.Time
  mutex racing.Mutex
- now *func() time.Time
  handledSecurityCodes collections.Set[string]
 }
 
@@ -73,7 +74,7 @@ func (m *neo4jAuthTokenManager) GetAuthToken(ctx context.Context) (auth.Token, e
  "could not acquire lock in time when getting token in neo4jAuthTokenManager")
  }
  defer m.mutex.Unlock()
- if m.token == nil || m.expiration != nil && (*m.now)().After(*m.expiration) {
+ if m.token == nil || m.expiration != nil && itime.Now().After(*m.expiration) {
  token, expiration, err := m.provider(ctx)
  if err != nil {
  return auth.Token{}, err
@@ -111,11 +112,9 @@ func (m *neo4jAuthTokenManager) HandleSecurityException(ctx context.Context, tok
 // The provider function must only ever return auth information belonging to the same identity.
 // Switching identities is undefined behavior.
 func BasicTokenManager(provider authTokenProvider) TokenManager {
- now := time.Now
  return &neo4jAuthTokenManager{
  provider: wrapWithNilExpiration(provider),
  mutex: racing.NewMutex(),
- now: &now,
  handledSecurityCodes: collections.NewSet([]string{
  "Neo.ClientError.Security.Unauthorized",
  }),
@@ -135,11 +134,9 @@ func BasicTokenManager(provider authTokenProvider) TokenManager {
 // The provider function must only ever return auth information belonging to the same identity.
 // Switching identities is undefined behavior.
 func BearerTokenManager(provider authTokenWithExpirationProvider) TokenManager {
- now := time.Now
  return &neo4jAuthTokenManager{
  provider: provider,
  mutex: racing.NewMutex(),
- now: &now,
  handledSecurityCodes: collections.NewSet([]string{
  "Neo.ClientError.Security.TokenExpired",
  "Neo.ClientError.Security.Unauthorized",

neo4j/config.go

@@ -19,6 +19,7 @@ package neo4j
 
 import (
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/config"
+ "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/pool"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/notifications"
  "math"
  "net/url"
@@ -41,6 +42,7 @@ func defaultConfig() *Config {
  MaxConnectionPoolSize: 100,
  MaxConnectionLifetime: 1 * time.Hour,
  ConnectionAcquisitionTimeout: 1 * time.Minute,
+ ConnectionLivenessCheckTimeout: pool.DefaultConnectionLivenessCheckTimeout,
  SocketConnectTimeout: 5 * time.Second,
  SocketKeepalive: true,
  RootCAs: nil,
@@ -77,6 +79,11 @@ func validateAndNormaliseConfig(config *Config) error {
  config.ConnectionAcquisitionTimeout = -1
  }
 
+ // Connection Liveness Check Timeout
+ if config.ConnectionLivenessCheckTimeout < 0 {
+ return &UsageError{Message: "Connection liveness check timeout cannot be smaller than 0"}
+ }
+
  // Socket Connect Timeout
  if config.SocketConnectTimeout < 0 {
  config.SocketConnectTimeout = 0

neo4j/config/driver.go

@@ -103,6 +103,21 @@ type Config struct {
  //
  // default: 1 * time.Minute
  ConnectionAcquisitionTimeout time.Duration
+ // ConnectionLivenessCheckTimeout sets the timeout duration for idle connections in the pool.
+ // Connections idle longer than this timeout will be tested for liveliness before reuse. A low timeout value
+ // can increase network requests when acquiring a connection, impacting performance. Conversely, a high
+ // timeout may result in using connections that are no longer active, causing exceptions in your application.
+ // These exceptions typically resolve with a retry or using a driver API with automatic
+ // retries, assuming the database is operational.
+ //
+ // The parameter balances the likelihood of encountering connection issues against performance.
+ // Typically, adjustment of this parameter is not necessary.
+ //
+ // By default, no liveliness check is performed. A value of 0 ensures connections are always tested for
+ // validity, and negative values are not permitted.
+ //
+ // default: pool.DefaultConnectionLivenessCheckTimeout
+ ConnectionLivenessCheckTimeout time.Duration
  // Connect timeout that will be set on underlying sockets. Values less than
  // or equal to 0 results in no timeout being applied.
  //

neo4j/config_test.go

@@ -99,6 +99,16 @@ func TestValidateAndNormaliseConfig(rt *testing.T) {
  }
  })
 
+ rt.Run("ConnectionLivenessCheckTimeout less than zero", func(t *testing.T) {
+ config := defaultConfig()
+
+ config.ConnectionLivenessCheckTimeout = -1 * time.Second
+ err := validateAndNormaliseConfig(config)
+ if err == nil {
+ t.Errorf("ConnectionLivenessCheckTimeout is less than 0 but never returned an error")
+ }
+ })
+
  rt.Run("SocketConnectTimeout less than zero", func(t *testing.T) {
  config := defaultConfig()
 

neo4j/driver_with_context.go

@@ -1,3 +1,5 @@
+// Package neo4j provides required functionality to connect and execute statements against a Neo4j Database.
+
 /*
  * Copyright (c) "Neo4j"
  * Neo4j Sweden AB [https://neo4j.com]
@@ -15,25 +17,22 @@
  * limitations under the License.
  */
 
-// Package neo4j provides required functionality to connect and execute statements against a Neo4j Database.
 package neo4j
 
 import (
  "context"
  "fmt"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/auth"
+ "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/connector"
  idb "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/db"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/errorutil"
+ "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/pool"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/racing"
+ "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/router"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/log"
  "net/url"
  "strings"
  "sync"
- "time"
-
- "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/connector"
- "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/pool"
- "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/router"
 )
 
 // AccessMode defines modes that routing driver decides to which cluster member
@@ -145,7 +144,7 @@ func NewDriverWithContext(target string, auth auth.TokenManager, configurers ...
  return nil, err
  }
 
- d := driverWithContext{target: parsed, mut: racing.NewMutex(), now: time.Now, auth: auth}
+ d := driverWithContext{target: parsed, mut: racing.NewMutex(), auth: auth}
 
  routing := true
  d.connector.Network = "tcp"
@@ -220,10 +219,9 @@ func NewDriverWithContext(target string, auth auth.TokenManager, configurers ...
  d.connector.Log = d.log
  d.connector.RoutingContext = routingContext
  d.connector.Config = d.config
- d.connector.Now = &d.now
 
  // Let the pool use the same log ID as the driver to simplify log reading.
- d.pool = pool.New(d.config, d.connector.Connect, d.log, d.logId, &d.now)
+ d.pool = pool.New(d.config, d.connector.Connect, d.log, d.logId)
 
  if !routing {
  d.router = &directRouter{address: address}
@@ -241,7 +239,15 @@ func NewDriverWithContext(target string, auth auth.TokenManager, configurers ...
  }
  }
  // Let the router use the same log ID as the driver to simplify log reading.
- d.router = router.New(address, routersResolver, routingContext, d.pool, d.log, d.logId, &d.now)
+ d.router = router.New(
+ address,
+ routersResolver,
+ routingContext,
+ d.pool,
+ d.config.ConnectionLivenessCheckTimeout,
+ d.log,
+ d.logId,
+ )
  }
 
  d.pool.SetRouter(d.router)
@@ -324,7 +330,6 @@ type driverWithContext struct {
  // this is *not* used by default by user-created session (see NewSession)
  executeQueryBookmarkManager BookmarkManager
  auth auth.TokenManager
- now func() time.Time
 }
 
 func (d *driverWithContext) Target() url.URL {
@@ -360,7 +365,7 @@ func (d *driverWithContext) NewSession(ctx context.Context, config SessionConfig
  return &erroredSessionWithContext{
  err: &UsageError{Message: "Trying to create session on closed driver"}}
  }
- return newSessionWithContext(d.config, config, d.router, d.pool, d.log, reAuthToken, &d.now)
+ return newSessionWithContext(d.config, config, d.router, d.pool, d.log, reAuthToken)
 }
 
 func (d *driverWithContext) VerifyConnectivity(ctx context.Context) error {

neo4j/driver_with_context_testkit.go

@@ -1,4 +1,4 @@
-//go:build internal_testkit
+//go:build internal_testkit && internal_time_mock
 
 /*
  * Copyright (c) "Neo4j"
@@ -25,22 +25,12 @@ import (
  idb "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/db"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/errorutil"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/router"
+ itime "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/time"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/log"
- "time"
 )
 
 type RoutingTable = idb.RoutingTable
 
-func SetTimer(d DriverWithContext, timer func() time.Time) {
- driver := d.(*driverWithContext)
- driver.now = timer
-}
-
-func ResetTime(d DriverWithContext) {
- driver := d.(*driverWithContext)
- driver.now = time.Now
-}
-
 func ForceRoutingTableUpdate(d DriverWithContext, database string, bookmarks []string, logger log.BoltLogger) error {
  driver := d.(*driverWithContext)
  ctx := context.Background()
@@ -70,3 +60,8 @@ func GetRoutingTable(d DriverWithContext, database string) (*RoutingTable, error
  table := router.GetTable(database)
  return table, nil
 }
+
+var Now = itime.Now
+var FreezeTime = itime.FreezeTime
+var TickTime = itime.TickTime
+var UnfreezeTime = itime.UnfreezeTime

neo4j/internal/bolt/bolt3.go

@@ -26,6 +26,7 @@ import (
  idb "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/db"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/errorutil"
  "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/telemetry"
+ itime "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/time"
  "net"
  "reflect"
  "time"
@@ -95,18 +96,16 @@ type bolt3 struct {
  authManager auth.TokenManager
  resetAuth bool
  errorListener ConnectionErrorListener
- now *func() time.Time
 }
 
 func NewBolt3(
  serverName string,
  conn net.Conn,
  errorListener ConnectionErrorListener,
- timer *func() time.Time,
  logger log.Logger,
  boltLog log.BoltLogger,
 ) *bolt3 {
- now := (*timer)()
+ now := itime.Now()
  b := &bolt3{
  state: bolt3_unauthorized,
  conn: conn,
@@ -123,7 +122,6 @@ func NewBolt3(
  idleDate: now,
  log: logger,
  errorListener: errorListener,
- now: timer,
  }
  b.out = &outgoing{
  chunker: newChunker(),
@@ -166,7 +164,7 @@ func (b *bolt3) receiveMsg(ctx context.Context) any {
  b.state = bolt3_dead
  return nil
  }
- b.idleDate = (*b.now)()
+ b.idleDate = itime.Now()
  return msg
 }
 

neo4j/internal/bolt/bolt3_test.go

@@ -104,7 +104,6 @@ func TestBolt3(outer *testing.T) {
  tcpConn, srv, cleanup := setupBolt3Pipe(t)
  go serverJob(srv)
 
- timer := time.Now
  c, err := Connect(
  context.Background(),
  "serverName",
@@ -116,7 +115,6 @@ func TestBolt3(outer *testing.T) {
  logger,
  nil,
  idb.NotificationConfig{},
- &timer,
  )
  if err != nil {
  t.Fatal(err)
@@ -158,7 +156,6 @@ func TestBolt3(outer *testing.T) {
  srv.waitForHello()
  srv.rejectHelloUnauthorized()
  }()
- timer := time.Now
  bolt, err := Connect(
  context.Background(),
  "serverName",
@@ -170,7 +167,6 @@ func TestBolt3(outer *testing.T) {
  logger,
  nil,
  idb.NotificationConfig{},
- &timer,
  )
  AssertNil(t, bolt)
  AssertError(t, err)