joellubi commented on issue #2062:
URL: https://github.com/apache/arrow-adbc/issues/2062#issuecomment-2270956770

   Hi @connelld-dpsk12 and @lidavidm. I've seen this issue recently as well and 
it appears to be upstream of us in gosnowflake. I opened an issue with them 
last week that provides some detail into what's happening: 
https://github.com/snowflakedb/gosnowflake/issues/1186.
   
   To summarize the bug, it appears that gosnowflake persists the `context` 
submitted with a query in the db connection it creates. Go specifically 
[warns](https://pkg.go.dev/database/sql/[email protected]#Connector) against this 
because the `database/sql` library automatically caches and pools these 
connections, so any associated context may be (and in this case is) invalid 
after it's been returned to the pool.
   
   The actual impact of this to us is unclear. The log emitted comes from 
[connection.go:275](https://github.com/snowflakedb/gosnowflake/blob/v1.10.1/connection.go#L275),
 which is the `Close()` method for a connection. The gosnowflake client 
attempts to make an API call to Snowflake informing it that the connection is 
closed, but does so with the invalid context. As far as I can tell, all our 
client-side code executes as expected and we release relevant resources on our 
end when we're done. I can only speculate the impact of not updating the SF 
server-state immediately; I imagine there's a timeout but it _might_ delay 
warehouse idling (just a guess).
   
   Ideally this gets fixed upstream, but there are a few options for how we can 
deal with this in the meantime:
   - Our primary `Execute[Query|Update]` methods manually manage a single 
connection and don't have this issue. The bug arises only when we use the 
`database/sql` API which creates a connection pool. The API is used for various 
auxiliary tasks (such as `COUNT(*)` to check rows_affected, query 
information_schema, etc) because it's a bit more convenient. If we use a fixed 
number of explicit connections for everything and stop using `database/sql`, we 
likely won't have this issue any more.
   - A slightly simpler variant of the above: We can call 
`db.SetMaxIdleConns(0)` after creating the `db` which seems to prevent pooling 
of any connections. This would be a smaller code change, but could increase 
time spent acquiring connections depending on the ADBC action being performed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to