CockroachDB may not properly support JDBC's isValid() API

[knz]

Reported by @theangryangel in theangryangel/logstash-output-jdbc#53 (comment)

Pertains to this line of original code: https://github.com/theangryangel/logstash-output-jdbc/blob/3dc76277827aa42cecfad0cbf4a6c3f7b33b6ea0/lib/logstash/outputs/jdbc.rb#L153

In short: logstash initializes the DB connection via JDBC, performs some initial configuration then calls Connection.isValid() to see if CockroachDB accepted the configuration. If the connection is OK it is returned back to the rest of the software for further use.

In the initial report isValid() was returning false.

My initial interpretation was that some configuration prior to the call to isValid wasn't recognized by CockroachDB, resulting in the server closing the connection (or something similar). However the code was since modified to bypass the call to isValid and apparently the connection can be used like this just fine (see theangryangel/logstash-output-jdbc@f46fd58) suggesting that isValid() itself may not be supported properly.

If so we need to investigate a small repro using JDBC and calling isValid() to see what really happens.

For reference the official doc for isValid is to be found here: http://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html?is-external=true#isValid-int-

Note in particular the doc specifies " The driver shall submit a query on the connection or use some other mechanism that positively verifies the connection is still valid when this method is called " which suggests some active support from the DB is required.

@theangryangel please chime in to confirm that my analysis is correct.

[bdarnell]

The implementation of isValid in the current postgres jdbc driver is here. It's preparing an empty statement and executing it (reusing the prepared statement over the life of the connection). I know we've had to add special-case support for sending the right response codes for empty statements; maybe we need something extra for preparing them.

[theangryangel]

@knz spot on from what I found during testing.

I had intended to take a look a cockroach and submit a patch for this myself (if I could figure out what needed to be done), but I've not managed to find the time yet - so thank you for spending time on my behalf ❤️

If there's anything I can do to help, let me know and I'll do what I can.

[maddyblue]

Prepare for empty statements is currently blocked on lib/pq#500, but the lib/pq devs have been absent for over a week.