SQL: jdbc debugging enhancement #53880
Conversation
after each debug message when enabled (disabled by default) * upon connection time and debug classes initialization, log in debug output information about OS, JVM and default JVM timezone
…o jdbc_debug_enhancement
|
Pinging @elastic/es-search (:Search/SQL) |
costin
left a comment
costin
left a comment
There was a problem hiding this comment.
Left a few comments regarding initialization.
| } | ||
|
|
||
| public void testDebugBuffered() throws Exception { | ||
| JdbcConfiguration ci = ci("jdbc:es://a:1/?debug=true&debug.buffered=false"); |
There was a problem hiding this comment.
I think debug.flushAlways or debug.noBuffer is more meaningful. I also like the consistency of the options - by default they are false and need to be turn on through true.
That is the default (false) is to always buffer but true would yield an immediate flush.
| private static final String HEADER = "%tF/%tT.%tL - "; | ||
|
|
||
| final PrintWriter print; | ||
| private boolean debugBuffered; |
There was a problem hiding this comment.
No need to keep the prefix debug as this is a DebugXXX class.
| log = new DebugLog(managedPrinter, info.debugBuffered()); | ||
| OUTPUT_MANAGED.put(managedPrinter, log); | ||
| } | ||
| return log; |
There was a problem hiding this comment.
The method changes make it more complicated and hard to follow - there's this log ==null being repeated which begs the question, what happens it if it initialized?
I think the method is good as it is - it creates a logger or returns one that already exists. Either wrap this method in another one that calls logSystemInfo (rename the existing method into something like createLogger and then logger calls createLogger().logSystemInfo or wrap that at the consumer site - inside proxy.
However currently the systemInfo is being called per each Connection which I think is excessive - we want the system information to be once per log, at the beginning.
In which case, the system info should called when a new log is created, essentially after each new DebugLog.
To keep things incapsulated, instead of calling the constructor, one could call a method createLog which internally calls new DebugLog and right after calls logSystemInfo.
This way any other initialization that would need to occur, would happen in that method regardless of the actual method taking place and only once per logger.
It might make sense to call flush after logging the system info.
…o jdbc_debug_enhancement
|
@costin very valid points. Pushed an update. |
* add flush always output option that will flush the output printer after each debug message when enabled (disabled by default) * at debug output initializationtime, log debug output information about OS, JVM and default JVM timezone (cherry picked from commit b5db965)
* add flush always output option that will flush the output printer after each debug message when enabled (disabled by default) * at debug output initializationtime, log debug output information about OS, JVM and default JVM timezone (cherry picked from commit b5db965)
* add flush always output option that will flush the output printer after each debug message when enabled (disabled by default) * at debug output initializationtime, log debug output information about OS, JVM and default JVM timezone (cherry picked from commit b5db965)
* add flush always output option that will flush the output printer after each debug message when enabled (disabled by default) * at debug output initializationtime, log debug output information about OS, JVM and default JVM timezone (cherry picked from commit b5db965)
5 participants
This change adds a buffered debug output option that will flush the output printer after each debug message when enabled (disabled by default). Also, upon connection time and debug classes initialization, an additional debug log message is generated that contains information about OS, JVM and default JVM timezone setting.