From b5201c836fb6fe14e9243e3e84a0dc462d875a16 Mon Sep 17 00:00:00 2001 From: Cathleen Yan Date: Tue, 18 Aug 2026 21:40:17 +0000 Subject: [PATCH] Fix JUL initialization after an OFF connection Signed-off-by: Cathleen Yan --- NEXT_CHANGELOG.md | 4 + .../com/databricks/jdbc/log/JulLogger.java | 66 ++++++++------- .../jdbc/common/util/LoggingUtilTest.java | 4 +- .../databricks/jdbc/log/JulLoggerTest.java | 84 ++++++++++++++++++- 4 files changed, 126 insertions(+), 32 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e12c8875f4..1e1b04f103 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -8,6 +8,10 @@ - `DatabaseMetaData.getColumns(...)` with a `null` catalog now issues a single `SHOW COLUMNS IN ALL CATALOGS` statement (consistent with `getSchemas`/`getTables`) instead of enumerating every catalog and issuing a per-catalog `SHOW COLUMNS`. Older DBR versions that do not support the syntax transparently fall back to the previous enumerate-and-fan-out behavior. ### Fixed +- Fixed later logging-enabled connections being unable to produce logs when an earlier connection + used `LogLevel=OFF`. The first enabled connection now establishes the shared JUL handler, while a + later `OFF` connection does not disable it. + - Fixed `IdleConnectionEvictor` thread leak in long-running applications. Driver-side resources (HTTP client, background threads) are now always released when `Connection.close()` is called, even if statement cleanup or server-side session termination fails. - Throw `DatabricksSQLException` instead of an unchecked `ClassCastException` when a complex-type getter (`getArray`, `getStruct`, `getMap`) is called on a column of a different complex type. diff --git a/src/main/java/com/databricks/jdbc/log/JulLogger.java b/src/main/java/com/databricks/jdbc/log/JulLogger.java index aa42097559..ba4ff8f336 100644 --- a/src/main/java/com/databricks/jdbc/log/JulLogger.java +++ b/src/main/java/com/databricks/jdbc/log/JulLogger.java @@ -114,7 +114,9 @@ public void error(Throwable throwable, String format, Object... arguments) { /** * Initializes the logger with the specified configuration. This method is synchronized to prevent - * concurrent modifications to the logger configuration. + * concurrent modifications to the logger configuration. A Level.OFF request suppresses JUL output + * without installing a handler, allowing a later enabled request to initialize the shared logger. + * Once an enabled handler is installed, subsequent requests do not reconfigure it. * * @param level the log level * @param logDir the directory for log files or {@code STDOUT} for console output @@ -124,35 +126,41 @@ public void error(Throwable throwable, String format, Object... arguments) { */ public static synchronized void initLogger( Level level, String logDir, int logFileSizeBytes, int logFileCount) throws IOException { - if (!isLoggerInitialized) { - isLoggerInitialized = true; - - // java.util.logging uses hierarchical loggers, so we just need to set the log level on the - // parent package logger. Using "com.databricks" as the prefix captures all JDBC driver - // classes as well as shaded dependencies (SDK, Apache HTTP client, etc.) - Logger jdbcJulLogger = Logger.getLogger(PARENT_CLASS_PREFIX); - jdbcJulLogger.setLevel(level); - jdbcJulLogger.setUseParentHandlers(false); - - String logPattern = getLogPattern(logDir); - Handler handler; - if (logPattern.equalsIgnoreCase(STDOUT)) { - handler = - new StreamHandler(System.out, new Slf4jFormatter()) { - @Override - public void publish(LogRecord record) { - super.publish(record); - // prompt flushing; full send >>> 🚀 - flush(); - } - }; - } else { - handler = new FileHandler(logPattern, logFileSizeBytes, logFileCount, true); - } - handler.setLevel(level); - handler.setFormatter(new Slf4jFormatter()); - jdbcJulLogger.addHandler(handler); + if (isLoggerInitialized) { + return; + } + + // java.util.logging uses hierarchical loggers, so we just need to set the log level on the + // parent package logger. Using "com.databricks" as the prefix captures all JDBC driver + // classes as well as shaded dependencies (SDK, Apache HTTP client, etc.) + Logger jdbcJulLogger = Logger.getLogger(PARENT_CLASS_PREFIX); + jdbcJulLogger.setUseParentHandlers(false); + + if (level == Level.OFF) { + jdbcJulLogger.setLevel(Level.OFF); + return; + } + + String logPattern = getLogPattern(logDir); + Handler handler; + if (logPattern.equalsIgnoreCase(STDOUT)) { + handler = + new StreamHandler(System.out, new Slf4jFormatter()) { + @Override + public void publish(LogRecord record) { + super.publish(record); + // prompt flushing; full send >>> 🚀 + flush(); + } + }; + } else { + handler = new FileHandler(logPattern, logFileSizeBytes, logFileCount, true); } + handler.setLevel(level); + handler.setFormatter(new Slf4jFormatter()); + jdbcJulLogger.addHandler(handler); + jdbcJulLogger.setLevel(level); + isLoggerInitialized = true; } private void log(Level level, String message, Throwable throwable) { diff --git a/src/test/java/com/databricks/jdbc/common/util/LoggingUtilTest.java b/src/test/java/com/databricks/jdbc/common/util/LoggingUtilTest.java index 6754f1cebe..2ed0feb98b 100644 --- a/src/test/java/com/databricks/jdbc/common/util/LoggingUtilTest.java +++ b/src/test/java/com/databricks/jdbc/common/util/LoggingUtilTest.java @@ -20,8 +20,8 @@ void testSetupLogger() { @Test void testSetupLoggerWithOffLevel() { - // When log level is OFF, setupLogger initializes logger with Level.OFF to suppress all output. - // It uses STDOUT to avoid file system access issues in restricted environments. + // When log level is OFF, setupLogger configures the parent logger with Level.OFF without + // creating a handler, leaving a later enabled connection free to initialize logging. // This should not throw an exception even if the log path is not writable. assertDoesNotThrow(() -> LoggingUtil.setupLogger("/", 1, 1, LogLevel.OFF)); assertDoesNotThrow(() -> LoggingUtil.setupLogger("/invalid/path", 1, 1, LogLevel.OFF)); diff --git a/src/test/java/com/databricks/jdbc/log/JulLoggerTest.java b/src/test/java/com/databricks/jdbc/log/JulLoggerTest.java index 515427f871..fe2b193b55 100644 --- a/src/test/java/com/databricks/jdbc/log/JulLoggerTest.java +++ b/src/test/java/com/databricks/jdbc/log/JulLoggerTest.java @@ -7,6 +7,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.concurrent.CompletableFuture; import java.util.logging.*; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -23,6 +24,7 @@ public class JulLoggerTest { @BeforeEach void setUp() { + resetLogger(); mockLogger = Mockito.mock(Logger.class); // By default treat every level as enabled so the existing verify-based tests // exercise the real logging path. Individual tests override this to assert the @@ -34,13 +36,19 @@ void setUp() { @AfterEach void tearDown() { - // Reset the logger after each test + resetLogger(); + } + + private void resetLogger() { JulLogger.isLoggerInitialized = false; Logger logger = Logger.getLogger(JulLogger.PARENT_CLASS_PREFIX); logger.setLevel(null); for (Handler handler : logger.getHandlers()) { logger.removeHandler(handler); + if (handler instanceof FileHandler) { + handler.close(); + } } logger.setUseParentHandlers(true); } @@ -172,6 +180,80 @@ void testInitLoggerWithFileHandler(@TempDir Path tempDir) throws IOException { } } + @Test + void testInitLoggerPromotesFromOffToEnabled(@TempDir Path tempDir) throws IOException { + Logger jdbcLogger = Logger.getLogger(JulLogger.PARENT_CLASS_PREFIX); + + JulLogger.initLogger(Level.OFF, JulLogger.STDOUT, 0, 0); + + assertEquals(Level.OFF, jdbcLogger.getLevel()); + assertEquals(0, jdbcLogger.getHandlers().length); + assertFalse(JulLogger.isLoggerInitialized); + + JulLogger.initLogger(Level.FINEST, tempDir.toString(), 1024, 1); + + assertEquals(Level.FINEST, jdbcLogger.getLevel()); + assertEquals(1, jdbcLogger.getHandlers().length); + assertInstanceOf(FileHandler.class, jdbcLogger.getHandlers()[0]); + assertTrue(Files.exists(tempDir.resolve(JulLogger.DATABRICKS_LOG_FILE))); + assertTrue(JulLogger.isLoggerInitialized); + } + + @Test + void testInitLoggerDoesNotDisableEnabledLogger(@TempDir Path tempDir) throws IOException { + Logger jdbcLogger = Logger.getLogger(JulLogger.PARENT_CLASS_PREFIX); + JulLogger.initLogger(Level.FINEST, tempDir.toString(), 1024, 1); + Handler enabledHandler = jdbcLogger.getHandlers()[0]; + + JulLogger.initLogger(Level.OFF, JulLogger.STDOUT, 0, 0); + + assertEquals(Level.FINEST, jdbcLogger.getLevel()); + assertArrayEquals(new Handler[] {enabledHandler}, jdbcLogger.getHandlers()); + assertTrue(JulLogger.isLoggerInitialized); + } + + @Test + void testConcurrentOffAndEnabledInitializationCreatesOneHandler(@TempDir Path tempDir) { + CompletableFuture offInitialization = + CompletableFuture.runAsync( + () -> + assertDoesNotThrow(() -> JulLogger.initLogger(Level.OFF, JulLogger.STDOUT, 0, 0))); + CompletableFuture enabledInitialization = + CompletableFuture.runAsync( + () -> + assertDoesNotThrow( + () -> JulLogger.initLogger(Level.INFO, tempDir.toString(), 1024, 1))); + + assertDoesNotThrow( + () -> CompletableFuture.allOf(offInitialization, enabledInitialization).join()); + + Logger jdbcLogger = Logger.getLogger(JulLogger.PARENT_CLASS_PREFIX); + assertEquals(Level.INFO, jdbcLogger.getLevel()); + assertEquals(1, jdbcLogger.getHandlers().length); + assertInstanceOf(FileHandler.class, jdbcLogger.getHandlers()[0]); + assertTrue(JulLogger.isLoggerInitialized); + } + + @Test + void testFailedInitializationCanBeRetried(@TempDir Path tempDir) throws IOException { + Path fileInsteadOfDirectory = tempDir.resolve("not-a-directory"); + Files.writeString(fileInsteadOfDirectory, "test"); + + assertThrows( + IOException.class, + () -> JulLogger.initLogger(Level.INFO, fileInsteadOfDirectory.toString(), 1024, 1)); + assertFalse(JulLogger.isLoggerInitialized); + + Path validLogDirectory = tempDir.resolve("logs"); + JulLogger.initLogger(Level.INFO, validLogDirectory.toString(), 1024, 1); + + Logger jdbcLogger = Logger.getLogger(JulLogger.PARENT_CLASS_PREFIX); + assertEquals(Level.INFO, jdbcLogger.getLevel()); + assertEquals(1, jdbcLogger.getHandlers().length); + assertTrue(Files.exists(validLogDirectory.resolve(JulLogger.DATABRICKS_LOG_FILE))); + assertTrue(JulLogger.isLoggerInitialized); + } + @Test void testGetCaller() { String[] caller = simulateLoggingCall();