[GitHub] [drill] paul-rogers commented on pull request #2333: DRILL-8009: DrillConnectionImpl#isValid() doesn't correspond JDBC API

[GitBox <gi...@apache.org>]

paul-rogers commented on pull request #2333:
URL: https://github.com/apache/drill/pull/2333#issuecomment-949284641


   Hi @rymarm, thanks for the contribution!
   
   Your description mentions that the Drill RPC already sends heartbeats. Here is some background.
   
   For systems such as MySQL, the connection to the server can remain open, but idle if no queries are sent. At some point, the MySql server will close the connection due to inactivity. The client won't know this until it tries to take some action. In this case, `isValid()` makes sense: it sends something to see if anyone is still on the other end.
   
   In Drill's case, there is always the heartbeat, so the connection will never timeout from disuse, only from a network failure or because the server failed. So, the client always knows that the connection is still valid (or was at the time of the last heartbeat. We can check how frequently they occur; I think it is on the order of 30 seconds or so, but I may be wrong.)
   
   The Drill JDBC connection uses Drill's native home-grown Netty-based RPC mechanism. That mechanism includes keep-alive heartbeats. On servers, each Drillbit sends the Ping/Pong messages to each other to verify that the Drillbits are still up. In fact, in the old days, somethings things would fail under heavy load because the CPU was so busy that the heartbeat thread didn't get a chance to run, and the connection was assumed failed.
   
   The Ping/Pong messages occur over the same channel as the data flow. We had cases where the client was slow to read data, and thus a heartbeat message was backed up behind data batches and the connection ended up failing as a result. I don't recall how we resolved that. Did we disable heartbeat for clients? Move heartbeat to the control channel?
   
   Given all this, the extra work in `isValid()` is not as necessary as it is with other systems such as MySQL. Still, no harm in adding an extra check. So, the question is, does the `isValid()` method need to know the state at this exact instant? Or, is it good enough to know that the connection was OK when we last checked 30 seconds (or whatever) ago? Either way, we'll keep checking in the background whether the client asks or not.
   
   What do you think?


-- 
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: dev-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org