Skip to content
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

SNOW-1563083: database/sql driver caches cancelled context #1186

Closed
joellubi opened this issue Jul 29, 2024 · 5 comments · Fixed by #1196
Closed

SNOW-1563083: database/sql driver caches cancelled context #1186

joellubi opened this issue Jul 29, 2024 · 5 comments · Fixed by #1196
Assignees
Labels
bug Erroneous or unexpected behaviour status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team

Comments

@joellubi
Copy link
Contributor

Please answer these questions before submitting your issue.
In order to accurately debug the issue this information is required. Thanks!

  1. What version of GO driver are you using?
    github.com/snowflakedb/gosnowflake v1.10.1

  2. What operating system and processor architecture are you using?

uname -mrsv 
Darwin 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:17:33 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6031 arm64
  1. What version of GO are you using?
    run go version in your console
go version    
go version go1.22.3 darwin/arm64
  1. Server version: 8.27.1

  2. What did you do?

If a *sql.DB instance is opened with sql.OpenDB(connector), then cancelling the context provided to db.QueryContext after the query has completed will result in warnings getting logged that the context is already closed when "recycling" the connection. I've reproduced a minimal example below, but the real-world scenario this comes up in is if errgroup is used to dispatch multiple queries concurrently. Once you Wait() for the group to finish, the group's context is cancelled. The problem is that the driver currently attempts to reuse this context in the call to defer db.Close() later when the function exits.

Based on my investigation, this seems like it has to do with the db connection pooling that Go's database/sql provides automatically. If you uncomment the line db.SetMaxIdleConns(0), then the error does not occur. I believe this is because it prevents Go from caching the connection.

In order for database/sql connections to be persist-able within the cache, they should not store the context from the query that spawned them since it may have a shorter lifetime than the connection if it is reused. The docs indicate this as well:

type Connector interface {
...
// The provided context.Context is for dialing purposes only
// (see net.DialContext) and should not be stored or used for
// other purposes.
...
Connect(context.Context) (Conn, error)
...
}

package main

import (
	"context"
	"database/sql"
	"fmt"
	"log"

	"github.com/snowflakedb/gosnowflake"
)

var uri = "REDACTED"

func main() {
	ctxCancel, cancel := context.WithCancel(context.Background())

	config, err := gosnowflake.ParseDSN(uri)
	if err != nil {
		log.Fatal(err)
	}

	connector := gosnowflake.NewConnector(gosnowflake.SnowflakeDriver{}, *config)
	db := sql.OpenDB(connector)
	defer db.Close()
	// db.SetMaxIdleConns(0)

	if err := printQueryResult(ctxCancel, db, "SELECT 'hello'"); err != nil {
		log.Fatal(err)
	}
	cancel()
}

func printQueryResult(ctx context.Context, db *sql.DB, query string) error {
	rows, err := db.QueryContext(ctx, query)
	if err != nil {
		return err
	}
	defer rows.Close()

	for rows.Next() {
		var val string
		if err := rows.Scan(&val); err != nil {
			return err
		}
		fmt.Println(val)
	}
	return nil
}
  1. What did you expect to see?

Expected

go run main.go
hello

Actual

go run main.go
hello
ERRO[0000]connection.go:275 gosnowflake.(*snowflakeConn).Close context canceled
  1. Can you set logging to DEBUG and collect the logs?

    https://community.snowflake.com/s/article/How-to-generate-log-file-on-Snowflake-connectors

  2. What is your Snowflake account identifier, if any? (Optional)

@joellubi joellubi added the bug Erroneous or unexpected behaviour label Jul 29, 2024
@github-actions github-actions bot changed the title database/sql driver caches cancelled context SNOW-1563083: database/sql driver caches cancelled context Jul 29, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage_done Initial triage done, will be further handled by the driver team label Jul 30, 2024
@sfc-gh-dszmolka
Copy link
Contributor

hi and thank you for raising this issue with us; additionally providing all these details and the reproduction is very much appreciated ! we'll take a look

@connelld-dpsk12
Copy link

hi and thank you for raising this issue with us; additionally providing all these details and the reproduction is very much appreciated ! we'll take a look

Any update @sfc-gh-dszmolka ?

@sfc-gh-dszmolka
Copy link
Contributor

I'm keeping all issues updated when there's progress on them - at this moment, i have no update. Issues are addressed by their respective teams in terms of the impact they have.
If this issue requires more attention and perhaps causing bigger impact for your company using Snowflake, please raise it with Snowflake Support too.

This repo is constantly monitored, however we cannot guarantee any specific timeline for resolution in advance. Thank you for your understanding!

@sfc-gh-dszmolka
Copy link
Contributor

#1196 merged - thank you for the contribution! will be released with the next upcoming driver release

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. label Aug 20, 2024
@sfc-gh-dszmolka
Copy link
Contributor

released with gosnowflake v.1.11.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Erroneous or unexpected behaviour status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants