denisenkom / go-mssqldb

Microsoft SQL server driver written in go language
BSD 3-Clause "New" or "Revised" License
1.82k stars 497 forks source link

Connect timeout affects all client requests #610

Open jlsimister-nice opened 3 years ago

jlsimister-nice commented 3 years ago

Describe the bug When the connect timeout is set on the connection string, it affects not only the time to connect to the server, but it affects every client request on the connection.

This can be problematic for queries, commands, and stored procs that take a long time on the server. They may time out due to the connection timeout (recommended default 15s) long before any client timeout (e.g., via the context on a QueryContext call).

On the one hand, the current behavior is a semi-convenient workaround for bug #609, but I don't believe it is correct behavior. The connect timeout should be separate from any query timeout (see https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/8313b90e-eee3-47e1-8441-73b3b5df1e39).

To Reproduce Run SQL Server on the local machine (default port 1433). You can use a separate machine if you modify the code below.

Execute the following go program, which sets a client timeout of 10s (via context), a connect timeout of 2s, and simulates a "long" query by executing a delay/sleep of 5s.

package main

import (
    "context"
    "database/sql"
    "fmt"
    mssql "github.com/denisenkom/go-mssqldb"
    "os"
    "time"
)

func main() {
    start := time.Now()
    ctx, cancel := context.WithDeadline(context.Background(), start.Add(time.Duration(10)*time.Second))
    defer cancel()

    connector, err := mssql.NewConnector("sqlserver://localhost:1433?connection+timeout=2")
    if err != nil {
        fmt.Println("NewConnector error:", err)
        os.Exit(1)
    }
    db := sql.OpenDB(connector)

    rows, err := db.QueryContext(ctx, "WAITFOR DELAY '00:00:05';")
    fmt.Println("Total QueryContext duration:", time.Since(start))
    if err != nil {
        fmt.Println("QueryContext error:", err)
        os.Exit(1)
    }
    defer rows.Close()
}

Expected behavior The driver should honor the query timeout from the context (10s in the example above) for individual commands/queries and complete the operation successfully. Commands should not be affected or cut short by the connect timeout specified on the connection string. The connect timeout should only govern the maximum time to establish a working connection.

Further technical details

SQL Server version: SQL Server 2008 R2 Operating system: Windows 10 (1903) Table schema: n/a

Additional context Related to bug #609.

prochac commented 3 years ago

For me, It causes timeout of BULK INSERT command what takes significant amount of time.