Skip to content

fix(bigquery-jdbc): use one logger per class to stop child-logger registration leak - #14716

Open
Neenu1995 wants to merge 5 commits into
mainfrom
neenu/fix-logger-leak
Open

Neenu1995 wants to merge 5 commits into
mainfrom
neenu/fix-logger-leak

Conversation

@Neenu1995

@Neenu1995 Neenu1995 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

b/576218509

  • Replace the per-instance LOG = new BigQueryJdbcCustomLogger(this.toString())
    field with one static final logger per class in BigQueryStatement,
    BigQueryConnection, BigQueryDatabaseMetaData, BigQueryCallableStatement,
    BigQueryPooledConnection (+ BigQueryPooledConnectionWrapper),
    PooledConnectionListener, and DataSource. Each logger is named after its
    own class so the CLASS column fallback in the formatter stays correct.
  • Remove the duplicate setParent(...) call in the 1-arg
    BigQueryJdbcCustomLogger constructor; the 2-arg constructor it delegates to
    already sets the parent. The second call forced a full scan of the parent's
    child list under the global treeLock on every construction.
  • Add regression tests to BigQueryJdbcCustomLoggerTest:
    • testLoggerIsParentedToDriverRootLogger locks in that every logger is
      still parented to the driver root logger (handlers/levels keep flowing).
    • testCreatingStatementsDoesNotRegisterChildLoggers and
      testCreatingDatabaseMetaDataDoesNotRegisterChildLoggers construct 1,000
      objects and assert the root logger's child count does not grow. Both fail
      on main and pass with this change.
  • Add --add-opens=java.logging/java.util.logging=ALL-UNNAMED to the surefire
    argLine (test-only) so the regression tests can read the JDK-internal
    child-logger list on JDK 17+.

No behavior change to per-connection log files or formatting: routing and the
[CONNECTION_ID] column come from the MDC / record parameters, not from the
logger name.

@Neenu1995
Neenu1995 requested review from a team as code owners October 21, 2026 16:20

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors BigQueryJdbcCustomLogger usage across several classes to be static final rather than instance-specific, preventing memory leaks caused by excessive child logger registrations. It also adds unit tests to verify that statement and metadata creation do not leak loggers, and updates the Maven configuration to permit reflective access to JDK-internal logger fields during testing. Feedback on the changes suggests avoiding the use of a hardcoded relative path (./logs) for test logging to prevent workspace pollution, recommending the system's temporary directory instead.

@Neenu1995

Copy link
Copy Markdown
Contributor Author

/gcprun

@logachev logachev added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 21, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 21, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants