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

added SessionResetter into driver_conn optional interfaces #1000

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

siddhesh-tamhanekar
Copy link

@siddhesh-tamhanekar siddhesh-tamhanekar commented Feb 11, 2025

Links

N/A

Details

Problem

The nrmysql driver in newrelic/go-agent does not properly handle connection reuse in case of last query was errored when MySQL closes an idle connection and application try to reuse it, This leads to a "commands out of sync" error when the same connection is used again.

Steps to Reproduce
The following test program demonstrates the issue:

package main

import (
	"database/sql"
	"fmt"
	"time"

	_ "github.com/go-sql-driver/mysql"
	_ "github.com/newrelic/go-agent/v3/integrations/nrmysql"
)

func main() {
	mysqlConnString := fmt.Sprintf("%s:%s@tcp(%s:%s)/%s?charset=utf8mb4&parseTime=True&loc=Local", "user", "pass", "127.0.0.1", "3306", "db_name")
	mdb, err := sql.Open("nrmysql", mysqlConnString)
	fmt.Println("mysql open error:", err)

	mdb.Ping()
	fmt.Println("ping error:", err)

	// This query will fail
	q("select * from table_not_exists", mdb)

	// Simulate MySQL closing the connection after idle time (e.g., 10s)
	time.Sleep(time.Second * 12)

	// This will result in "commands out of sync" error
	q("select * from users", mdb)
}

func q(sql string, mdb *sql.DB) {
	rows, err := mdb.Query(sql)
	if err == nil {
		cols, _ := rows.Columns()
		columns := make([]interface{}, len(cols))
		columnPointers := make([]interface{}, len(cols))
		for i := range columns {
			columnPointers[i] = &columns[i]
		}
		rows.Next()
		scanErr := rows.Scan(columnPointers...)
		fmt.Println("mysql query error:", err, scanErr, columns)
	} else {
		fmt.Println("mysql query error:", err)
	}
}

Root Cause

The repository provides an SQL driver that implements various sql package interfaces. One of these interfaces is driver.SessionResetter, which is already implemented in sql_driver.go. However, the optionalMethodsConn type in sql_driver_optional_methods.go did not support driver.SessionResetter.

Solution

  • Updated internal/tools/interface-wrapping to include SessionResetter.
  • Regenerated optionalMethodsConn to ensure SessionResetter is properly implemented.
  • This allows MySQL driver resetSession method to be called to determine whether the connection is still valid before reuse.

Additional Notes

  • The interface-wrapping tool was updated to recognize SessionResetter.
  • optionalMethodsConn was regenerated to reflect this change.
  • Tests have been validated to ensure correctness.
  • No breaking changes introduced.

@CLAassistant
Copy link

CLAassistant commented Feb 11, 2025

CLA assistant check
All committers have signed the CLA.

@siddhesh-tamhanekar siddhesh-tamhanekar force-pushed the hotfix/adding-SessionResetter-support branch from b2764dd to 0ee350b Compare February 11, 2025 14:32
@siddhesh-tamhanekar siddhesh-tamhanekar changed the base branch from master to develop February 23, 2025 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants