search

bippio / go-impala

Golang Driver for Apache Impala
MIT License
51 stars 40 forks source link

go-impala doesn't close queries #7

Closed timarmstrong closed 5 years ago

timarmstrong commented 5 years ago

Well-behaved clients are expected to close queries via the close() method when they are done with them: https://github.com/apache/impala/blob/master/common/thrift/beeswax.thrift#L179. This releases all resources associated with the queries and generally avoids open queries from accumulating on the server.

I experimented with impala-go and it leaves all of the queries dangling until the client disconnects. I0211 08:43:31.541136 30975 impala-server.cc:2012] Connection from client ::ffff:127.0.0.1:38950 closed, closing 1 associated session(s) I0211 08:43:31.541154 30975 impala-server.cc:1260] Closing session: d94d857aa1bb0ada:8732f4f69745269b I0211 08:43:31.541177 30975 impala-server.cc:1142] UnregisterQuery(): query_id=f84b83f4b6db701a:7457100f00000000 I0211 08:43:31.541182 30975 impala-server.cc:1249] Cancel(): query_id=f84b83f4b6db701a:7457100f00000000 I0211 08:43:31.541781 30975 impala-server.cc:1142] UnregisterQuery(): query_id=87424b90dbea4d1f:7796c34b00000000 I0211 08:43:31.541787 30975 impala-server.cc:1249] Cancel(): query_id=87424b90dbea4d1f:7796c34b00000000 I0211 08:43:31.542315 30975 impala-server.cc:1142] UnregisterQuery(): query_id=d24932ba436cdf1c:78fc87f000000000 I0211 08:43:31.542320 30975 impala-server.cc:1249] Cancel(): query_id=d24932ba436cdf1c:78fc87f000000000

I don't see any calls to close() in the source code of impala-go which explains that.

Unfortunately the contract around query unregistration is a little messy right now - in some cases, if the query encounters an error, it will return the error to the client on the next RPC and then automatically close the query. This only happens in some cases. I'd recommend calling close() and ignoring any QueryNotFoundException returned by close()

timarmstrong commented 5 years ago

There's an equivalent HS2 method as well.

vishjosh commented 5 years ago

@intamyuto @paribhasha : For you to fix , depending on who gets to this first

shaloi commented 5 years ago

@vishjosh I will have a look at this issue.

shaloi commented 5 years ago

@intamyuto since you are already looking at the query error scenarios, can you please have a look at this issue as well? thanks.

shaloi commented 5 years ago

fixed via #11