From d46382798a201dc9634f2e3239ccc24fb30327de Mon Sep 17 00:00:00 2001 From: Florent Biville Date: Fri, 27 Jan 2023 16:16:29 +0300 Subject: [PATCH] Remove read timeout log from critical section --- neo4j/internal/bolt/bolt3.go | 3 -- neo4j/internal/bolt/bolt3server_test.go | 3 +- neo4j/internal/bolt/bolt4.go | 3 -- neo4j/internal/bolt/bolt4server_test.go | 3 +- neo4j/internal/bolt/bolt5.go | 3 -- neo4j/internal/bolt/bolt5server_test.go | 3 +- neo4j/internal/bolt/chunker_test.go | 3 +- neo4j/internal/bolt/dechunker.go | 38 +++----------------- neo4j/internal/bolt/dechunker_test.go | 9 +++-- neo4j/internal/bolt/hydratedehydrate_test.go | 3 +- neo4j/internal/bolt/incoming.go | 6 +--- neo4j/internal/bolt/outgoing_test.go | 3 +- 12 files changed, 16 insertions(+), 64 deletions(-) diff --git a/neo4j/internal/bolt/bolt3.go b/neo4j/internal/bolt/bolt3.go index cbbc09b0..91d06fe7 100644 --- a/neo4j/internal/bolt/bolt3.go +++ b/neo4j/internal/bolt/bolt3.go @@ -100,8 +100,6 @@ func NewBolt3(serverName string, conn net.Conn, logger log.Logger, boltLog log.B boltMajor: 3, }, connReadTimeout: -1, - logger: logger, - logName: log.Bolt3, }, birthDate: now, idleDate: now, @@ -207,7 +205,6 @@ func (b *bolt3) Connect(ctx context.Context, minor int, auth map[string]any, use b.connId = succ.connectionId connectionLogId := fmt.Sprintf("%s@%s", b.connId, b.serverName) b.logId = connectionLogId - b.in.logId = connectionLogId b.in.hyd.logId = connectionLogId b.out.logId = connectionLogId b.serverVersion = succ.server diff --git a/neo4j/internal/bolt/bolt3server_test.go b/neo4j/internal/bolt/bolt3server_test.go index d5ed97c5..24f252eb 100644 --- a/neo4j/internal/bolt/bolt3server_test.go +++ b/neo4j/internal/bolt/bolt3server_test.go @@ -22,7 +22,6 @@ package bolt import ( "context" "fmt" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "io" "net" "testing" @@ -93,7 +92,7 @@ func (s *bolt3server) waitForHello() { } func (s *bolt3server) receiveMsg() *testStruct { - _, buf, err := dechunkMessage(context.Background(), s.conn, []byte{}, -1, log.Void{}, "", "") + _, buf, err := dechunkMessage(context.Background(), s.conn, []byte{}, -1) if err != nil { panic(err) } diff --git a/neo4j/internal/bolt/bolt4.go b/neo4j/internal/bolt/bolt4.go index d206f5f9..df2a8035 100644 --- a/neo4j/internal/bolt/bolt4.go +++ b/neo4j/internal/bolt/bolt4.go @@ -118,8 +118,6 @@ func NewBolt4(serverName string, conn net.Conn, logger log.Logger, boltLog log.B boltMajor: 4, }, connReadTimeout: -1, - logger: logger, - logName: log.Bolt4, }, lastQid: -1, } @@ -276,7 +274,6 @@ func (b *bolt4) Connect(ctx context.Context, minor int, auth map[string]any, use connectionLogId := fmt.Sprintf("%s@%s", b.connId, b.serverName) b.logId = connectionLogId b.in.hyd.logId = connectionLogId - b.in.logId = connectionLogId b.out.logId = connectionLogId b.initializeReadTimeoutHint(succ.configurationHints) diff --git a/neo4j/internal/bolt/bolt4server_test.go b/neo4j/internal/bolt/bolt4server_test.go index 34cfe85f..a655a399 100644 --- a/neo4j/internal/bolt/bolt4server_test.go +++ b/neo4j/internal/bolt/bolt4server_test.go @@ -22,7 +22,6 @@ package bolt import ( "context" "fmt" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "io" "net" "reflect" @@ -106,7 +105,7 @@ func (s *bolt4server) waitForHello() map[string]any { } func (s *bolt4server) receiveMsg() *testStruct { - _, buf, err := dechunkMessage(context.Background(), s.conn, []byte{}, -1, log.Void{}, "", "") + _, buf, err := dechunkMessage(context.Background(), s.conn, []byte{}, -1) if err != nil { panic(err) } diff --git a/neo4j/internal/bolt/bolt5.go b/neo4j/internal/bolt/bolt5.go index 7c0dadf0..73a4da24 100644 --- a/neo4j/internal/bolt/bolt5.go +++ b/neo4j/internal/bolt/bolt5.go @@ -118,8 +118,6 @@ func NewBolt5(serverName string, conn net.Conn, logger log.Logger, boltLog log.B useUtc: true, }, connReadTimeout: -1, - logger: logger, - logName: log.Bolt5, }, lastQid: -1, } @@ -265,7 +263,6 @@ func (b *bolt5) Connect(ctx context.Context, minor int, auth map[string]any, use connectionLogId := fmt.Sprintf("%s@%s", b.connId, b.serverName) b.logId = connectionLogId b.in.hyd.logId = connectionLogId - b.in.logId = connectionLogId b.out.logId = connectionLogId b.initializeReadTimeoutHint(succ.configurationHints) diff --git a/neo4j/internal/bolt/bolt5server_test.go b/neo4j/internal/bolt/bolt5server_test.go index 3867e2ae..b19f190d 100644 --- a/neo4j/internal/bolt/bolt5server_test.go +++ b/neo4j/internal/bolt/bolt5server_test.go @@ -22,7 +22,6 @@ package bolt import ( "context" "fmt" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "io" "net" "testing" @@ -96,7 +95,7 @@ func (s *bolt5server) waitForHello() map[string]any { } func (s *bolt5server) receiveMsg() *testStruct { - _, buf, err := dechunkMessage(context.Background(), s.conn, []byte{}, -1, log.Void{}, "", "") + _, buf, err := dechunkMessage(context.Background(), s.conn, []byte{}, -1) if err != nil { panic(err) } diff --git a/neo4j/internal/bolt/chunker_test.go b/neo4j/internal/bolt/chunker_test.go index 02019888..ab02c090 100644 --- a/neo4j/internal/bolt/chunker_test.go +++ b/neo4j/internal/bolt/chunker_test.go @@ -22,7 +22,6 @@ package bolt import ( "bytes" "context" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "net" "testing" @@ -83,7 +82,7 @@ func TestChunker(ot *testing.T) { receiveAndAssertMessage := func(t *testing.T, conn net.Conn, expected []byte) { t.Helper() - _, msg, err := dechunkMessage(context.Background(), conn, []byte{}, -1, log.Void{}, "", "") + _, msg, err := dechunkMessage(context.Background(), conn, []byte{}, -1) AssertNoError(t, err) assertSlices(t, msg, expected) } diff --git a/neo4j/internal/bolt/dechunker.go b/neo4j/internal/bolt/dechunker.go index 26ea3c93..015b885c 100644 --- a/neo4j/internal/bolt/dechunker.go +++ b/neo4j/internal/bolt/dechunker.go @@ -23,7 +23,6 @@ import ( "context" "encoding/binary" rio "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/racing" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "net" "time" ) @@ -34,14 +33,7 @@ import ( // Reads will race against the provided context ctx // If the server provides the connection read timeout hint readTimeout, a new context will be created from that timeout // and the user-provided context ctx before every read -func dechunkMessage( - ctx context.Context, - conn net.Conn, - msgBuf []byte, - readTimeout time.Duration, - logger log.Logger, - logName string, - logId string) ([]byte, []byte, error) { +func dechunkMessage(ctx context.Context, conn net.Conn, msgBuf []byte, readTimeout time.Duration) ([]byte, []byte, error) { sizeBuf := []byte{0x00, 0x00} off := 0 @@ -49,7 +41,7 @@ func dechunkMessage( reader := rio.NewRacingReader(conn) for { - updatedCtx, cancelFunc := newContext(ctx, readTimeout, logger, logName, logId) + updatedCtx, cancelFunc := newContext(ctx, readTimeout) _, err := reader.ReadFull(updatedCtx, sizeBuf) if err != nil { return msgBuf, nil, processReadError(err, ctx, readTimeout) @@ -73,7 +65,7 @@ func dechunkMessage( msgBuf = newMsgBuf } // Read the chunk into buffer - updatedCtx, cancelFunc = newContext(ctx, readTimeout, logger, logName, logId) + updatedCtx, cancelFunc = newContext(ctx, readTimeout) _, err = reader.ReadFull(updatedCtx, msgBuf[off:(off+chunkSize)]) if err != nil { return msgBuf, nil, processReadError(err, ctx, readTimeout) @@ -86,21 +78,9 @@ func dechunkMessage( } // newContext computes a new context and cancel function if a readTimeout is set -func newContext( - ctx context.Context, - readTimeout time.Duration, - logger log.Logger, - logName string, - logId string) (context.Context, context.CancelFunc) { - +func newContext(ctx context.Context, readTimeout time.Duration) (context.Context, context.CancelFunc) { if readTimeout >= 0 { - newCtx, cancelFunc := context.WithTimeout(ctx, readTimeout) - logger.Debugf(logName, logId, - "read timeout of %s applied, chunk read deadline is now: %s", - readTimeout.String(), - deadlineOf(newCtx), - ) - return newCtx, cancelFunc + return context.WithTimeout(ctx, readTimeout) } return ctx, nil } @@ -120,11 +100,3 @@ func processReadError(err error, ctx context.Context, readTimeout time.Duration) } return err } - -func deadlineOf(ctx context.Context) string { - deadline, hasDeadline := ctx.Deadline() - if !hasDeadline { - return "N/A (no deadline set)" - } - return deadline.String() -} diff --git a/neo4j/internal/bolt/dechunker_test.go b/neo4j/internal/bolt/dechunker_test.go index 8d2c456d..9d4eada4 100644 --- a/neo4j/internal/bolt/dechunker_test.go +++ b/neo4j/internal/bolt/dechunker_test.go @@ -23,7 +23,6 @@ import ( "bytes" "context" "encoding/binary" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "net" "reflect" "testing" @@ -83,7 +82,7 @@ func TestDechunker(t *testing.T) { go func() { AssertWriteSucceeds(t, cli, str.Bytes()) }() - buf, msgBuf, err = dechunkMessage(context.Background(), serv, buf, -1, log.Void{}, "", "") + buf, msgBuf, err = dechunkMessage(context.Background(), serv, buf, -1) AssertNoError(t, err) AssertLen(t, msgBuf, int(msg.size)) // Check content of buffer @@ -123,7 +122,7 @@ func TestDechunkerWithTimeout(ot *testing.T) { AssertWriteSucceeds(t, cli, []byte{0x00, 0x00}) }() buffer := make([]byte, 2) - _, _, err := dechunkMessage(context.Background(), serv, buffer, timeout, log.Void{}, "", "") + _, _, err := dechunkMessage(context.Background(), serv, buffer, timeout) AssertNoError(t, err) AssertTrue(t, reflect.DeepEqual(buffer, []byte{0xCA, 0xFE})) }) @@ -132,7 +131,7 @@ func TestDechunkerWithTimeout(ot *testing.T) { serv, cli := net.Pipe() defer closePipe(ot, serv, cli) - _, _, err := dechunkMessage(context.Background(), serv, nil, timeout, log.Void{}, "", "") + _, _, err := dechunkMessage(context.Background(), serv, nil, timeout) AssertError(t, err) AssertStringContain(t, err.Error(), "context deadline exceeded") @@ -144,7 +143,7 @@ func TestDechunkerWithTimeout(ot *testing.T) { ctx, cancelFunc := context.WithTimeout(context.Background(), timeout) defer cancelFunc() - _, _, err := dechunkMessage(ctx, serv, nil, -1, log.Void{}, "", "") + _, _, err := dechunkMessage(ctx, serv, nil, -1) AssertError(t, err) AssertStringContain(t, err.Error(), "context deadline exceeded") diff --git a/neo4j/internal/bolt/hydratedehydrate_test.go b/neo4j/internal/bolt/hydratedehydrate_test.go index 09e57778..ad735a04 100644 --- a/neo4j/internal/bolt/hydratedehydrate_test.go +++ b/neo4j/internal/bolt/hydratedehydrate_test.go @@ -21,7 +21,6 @@ package bolt import ( "context" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "net" "testing" "time" @@ -58,7 +57,7 @@ func TestDehydrateHydrate(ot *testing.T) { go func() { out.send(context.Background(), cli) }() - _, byts, err := dechunkMessage(context.Background(), serv, []byte{}, -1, log.Void{}, "", "") + _, byts, err := dechunkMessage(context.Background(), serv, []byte{}, -1) if err != nil { t.Fatal(err) } diff --git a/neo4j/internal/bolt/incoming.go b/neo4j/internal/bolt/incoming.go index b5376ed0..9800cfcf 100644 --- a/neo4j/internal/bolt/incoming.go +++ b/neo4j/internal/bolt/incoming.go @@ -21,7 +21,6 @@ package bolt import ( "context" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "net" "time" ) @@ -30,16 +29,13 @@ type incoming struct { buf []byte // Reused buffer hyd hydrator connReadTimeout time.Duration - logger log.Logger - logName string - logId string } func (i *incoming) next(ctx context.Context, rd net.Conn) (any, error) { // Get next message from transport layer var err error var msg []byte - i.buf, msg, err = dechunkMessage(ctx, rd, i.buf, i.connReadTimeout, i.logger, i.logName, i.logId) + i.buf, msg, err = dechunkMessage(ctx, rd, i.buf, i.connReadTimeout) if err != nil { return nil, err } diff --git a/neo4j/internal/bolt/outgoing_test.go b/neo4j/internal/bolt/outgoing_test.go index 6e63d19e..43eae6d4 100644 --- a/neo4j/internal/bolt/outgoing_test.go +++ b/neo4j/internal/bolt/outgoing_test.go @@ -22,7 +22,6 @@ package bolt import ( "context" idb "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/db" - "github.com/neo4j/neo4j-go-driver/v5/neo4j/log" "net" "reflect" "testing" @@ -112,7 +111,7 @@ func TestOutgoing(ot *testing.T) { }() // Dechunk it - _, byts, err := dechunkMessage(context.Background(), serv, []byte{}, -1, log.Void{}, "", "") + _, byts, err := dechunkMessage(context.Background(), serv, []byte{}, -1) if err != nil { t.Fatal(err) }