Skip to content

Commit e83c768

Browse files
fix(bigquery-jdbc): stop per-ResultSet logger registration leak (#14718)
b/576218509 One registered BigQueryJdbcCustomLogger per class, shared; the facade holds that delegate plus the connection ID and stamps the ID into LogRecord params[0] exactly as before, so per-connection file routing for un-proxied ResultSet calls is unchanged. Hot-path trace methods still set the source class/method explicitly and allocate nothing when the level is disabled. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 1b7d00d commit e83c768

2 files changed

Lines changed: 216 additions & 133 deletions

File tree

‎java-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcResultSetLogger.java‎

Lines changed: 93 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -22,36 +22,73 @@
2222
import java.util.logging.LogRecord;
2323

2424
/**
25-
* Specialized high-performance logger for ResultSet and its related classes. Avoids expensive stack
26-
* trace walking (caller inference) on hot-path logging calls.
25+
* High-performance logging facade for ResultSet and its related classes.
26+
*
27+
* <p>This is deliberately <em>not</em> a {@link java.util.logging.Logger}. Every {@code Logger}
28+
* instance registers itself as a child of its parent under a JVM-wide lock and that registration is
29+
* never released, so a logger-per-ResultSet leaks. Instead, one real {@link
30+
* BigQueryJdbcCustomLogger} is created per class and shared; this facade holds a reference to it
31+
* plus the connection ID of the ResultSet it serves.
32+
*
33+
* <p>Hot-path methods ({@code finestTrace} etc.) set the source class and method explicitly, so no
34+
* stack-trace walking (caller inference) happens for them. The connection ID is stamped into {@code
35+
* LogRecord.getParameters()[0]} on every record so {@code PerConnectionFileHandler} can route the
36+
* record even when the thread-local MDC (mapped diagnostic context — the per-thread connection ID
37+
* set by the JDBC proxy) is empty, which is the normal case for un-proxied ResultSet calls.
2738
*/
28-
public class BigQueryJdbcResultSetLogger extends BigQueryJdbcCustomLogger {
29-
private static final ConcurrentHashMap<String, BigQueryJdbcResultSetLogger> cache =
39+
public final class BigQueryJdbcResultSetLogger {
40+
41+
/** One registered logger per class. Bounded by the number of result-set classes. */
42+
private static final ConcurrentHashMap<String, BigQueryJdbcCustomLogger> DELEGATES =
43+
new ConcurrentHashMap<>();
44+
45+
/** Facades with no connection ID are immutable and shared, matching the old caching behaviour. */
46+
private static final ConcurrentHashMap<String, BigQueryJdbcResultSetLogger> GLOBAL_FACADES =
3047
new ConcurrentHashMap<>();
3148

49+
private final BigQueryJdbcCustomLogger delegate;
3250
private final String targetClassName;
3351
private final String connectionId;
3452

53+
private volatile BigQueryJdbcResultSetLogger arrowStructLogger;
54+
private volatile BigQueryJdbcResultSetLogger jsonStructLogger;
55+
private volatile BigQueryJdbcResultSetLogger arrowArrayLogger;
56+
private volatile BigQueryJdbcResultSetLogger jsonArrayLogger;
57+
58+
private BigQueryJdbcResultSetLogger(
59+
BigQueryJdbcCustomLogger delegate, String targetClassName, String connectionId) {
60+
this.delegate = delegate;
61+
this.targetClassName = targetClassName;
62+
this.connectionId = connectionId;
63+
}
64+
3565
public static BigQueryJdbcResultSetLogger getLogger(Class<?> clazz) {
3666
return getLogger(clazz, null);
3767
}
3868

3969
public static BigQueryJdbcResultSetLogger getLogger(Class<?> clazz, String connectionId) {
70+
String className = clazz.getName();
71+
BigQueryJdbcCustomLogger delegate =
72+
DELEGATES.computeIfAbsent(className, BigQueryJdbcCustomLogger::new);
4073
if (connectionId == null) {
41-
return cache.computeIfAbsent(
42-
clazz.getName(), k -> new BigQueryJdbcResultSetLogger(clazz, null));
74+
return GLOBAL_FACADES.computeIfAbsent(
75+
className, k -> new BigQueryJdbcResultSetLogger(delegate, k, null));
4376
}
44-
return new BigQueryJdbcResultSetLogger(clazz, connectionId);
77+
return new BigQueryJdbcResultSetLogger(delegate, className, connectionId);
4578
}
4679

4780
public String getConnectionId() {
4881
return connectionId;
4982
}
5083

51-
private BigQueryJdbcResultSetLogger arrowStructLogger;
52-
private BigQueryJdbcResultSetLogger jsonStructLogger;
53-
private BigQueryJdbcResultSetLogger arrowArrayLogger;
54-
private BigQueryJdbcResultSetLogger jsonArrayLogger;
84+
/** The shared, registered logger this facade forwards to. Exposed for tests only. */
85+
BigQueryJdbcCustomLogger delegate() {
86+
return delegate;
87+
}
88+
89+
public boolean isLoggable(Level level) {
90+
return delegate.isLoggable(level);
91+
}
5592

5693
public BigQueryJdbcResultSetLogger getArrowStructLogger() {
5794
if (arrowStructLogger == null) {
@@ -81,20 +118,13 @@ public BigQueryJdbcResultSetLogger getJsonArrayLogger() {
81118
return jsonArrayLogger;
82119
}
83120

84-
public BigQueryJdbcResultSetLogger(Class<?> clazz) {
85-
this(clazz, null);
86-
}
87-
88-
public BigQueryJdbcResultSetLogger(Class<?> clazz, String connectionId) {
89-
super(clazz.getName());
90-
this.targetClassName = clazz.getName();
91-
this.connectionId = connectionId;
92-
}
93-
94-
@Override
121+
/**
122+
* Stamps the connection ID into {@code params[0]} and forwards to the shared logger. Does not
123+
* touch the record's source class/method: hot-path callers set them explicitly, and cold-path
124+
* callers ({@code severe}, {@code warning}) rely on {@link
125+
* BigQueryJdbcCustomLogger.BigQueryJdbcLogRecord} to infer them lazily.
126+
*/
95127
public void log(LogRecord record) {
96-
record.setSourceClassName(targetClassName);
97-
record.setThreadID((int) Thread.currentThread().getId());
98128
if (connectionId != null) {
99129
Object[] existingParams = record.getParameters();
100130
if (existingParams == null || existingParams.length == 0) {
@@ -106,93 +136,104 @@ public void log(LogRecord record) {
106136
record.setParameters(newParams);
107137
}
108138
}
109-
super.log(record);
139+
delegate.log(record);
110140
}
111141

112142
private void logTrace(Level level, String methodName, String msg) {
113-
if (isLoggable(level)) {
114-
LogRecord record = new LogRecord(level, msg);
115-
record.setSourceMethodName(methodName);
116-
this.log(record);
143+
if (!delegate.isLoggable(level)) {
144+
return;
117145
}
146+
LogRecord record = new LogRecord(level, msg);
147+
record.setSourceClassName(targetClassName);
148+
record.setSourceMethodName(methodName);
149+
log(record);
118150
}
119151

120152
private void logTrace(Level level, String methodName, Supplier<String> msgSupplier) {
121-
if (isLoggable(level)) {
122-
LogRecord record = new LogRecord(level, msgSupplier.get());
123-
record.setSourceMethodName(methodName);
124-
this.log(record);
153+
if (!delegate.isLoggable(level)) {
154+
return;
125155
}
156+
logTrace(level, methodName, msgSupplier.get());
126157
}
127158

128-
/**
129-
* Log a message at Level.FINEST with predefined class name, method name and messageto avoid stack
130-
* trace parsing on the hot-path.
131-
*/
159+
/** Cold path: builds a record whose caller is inferred lazily by the formatter. */
160+
private void logWithCaller(Level level, Throwable thrown, String msg) {
161+
if (!delegate.isLoggable(level)) {
162+
return;
163+
}
164+
LogRecord record = new BigQueryJdbcCustomLogger.BigQueryJdbcLogRecord(level, msg);
165+
record.setThrown(thrown);
166+
log(record);
167+
}
168+
169+
/** Log at FINEST with the class name, method name and "++enter++" preset (no stack walk). */
132170
public void finestTrace(String methodName) {
133171
logTrace(Level.FINEST, methodName, "++enter++");
134172
}
135173

136-
/**
137-
* Log a message at Level.FINEST with predefined class name and method name to avoid stack trace
138-
* parsing on the hot-path.
139-
*/
174+
/** Log at FINEST with the class name and method name preset (no stack walk). */
140175
public void finestTrace(String methodName, String msg) {
141176
logTrace(Level.FINEST, methodName, msg);
142177
}
143178

144-
/** Log a lazy message at Level.FINEST with predefined class name and method name. */
179+
/** Log a lazy message at FINEST with the class name and method name preset. */
145180
public void finestTrace(String methodName, Supplier<String> msgSupplier) {
146181
logTrace(Level.FINEST, methodName, msgSupplier);
147182
}
148183

149-
/** Log a message at Level.FINER with predefined class name and method name. */
184+
/** Log at FINER with the class name and method name preset. */
150185
public void finerTrace(String methodName, String msg) {
151186
logTrace(Level.FINER, methodName, msg);
152187
}
153188

154-
/** Log a lazy message at Level.FINER with predefined class name and method name. */
189+
/** Log a lazy message at FINER with the class name and method name preset. */
155190
public void finerTrace(String methodName, Supplier<String> msgSupplier) {
156191
logTrace(Level.FINER, methodName, msgSupplier);
157192
}
158193

159-
/** Log a message at Level.FINE with predefined class name and method name. */
194+
/** Log at FINE with the class name and method name preset. */
160195
public void fineTrace(String methodName, String msg) {
161196
logTrace(Level.FINE, methodName, msg);
162197
}
163198

164-
/** Log a lazy message at Level.FINE with predefined class name and method name. */
199+
/** Log a lazy message at FINE with the class name and method name preset. */
165200
public void fineTrace(String methodName, Supplier<String> msgSupplier) {
166201
logTrace(Level.FINE, methodName, msgSupplier);
167202
}
168203

169-
@Override
170204
public void finest(String msg) {
171205
logTrace(Level.FINEST, "unknown", msg);
172206
}
173207

174-
@Override
175208
public void finest(Supplier<String> msgSupplier) {
176209
logTrace(Level.FINEST, "unknown", msgSupplier);
177210
}
178211

179-
@Override
180212
public void finer(String msg) {
181213
logTrace(Level.FINER, "unknown", msg);
182214
}
183215

184-
@Override
185216
public void finer(Supplier<String> msgSupplier) {
186217
logTrace(Level.FINER, "unknown", msgSupplier);
187218
}
188219

189-
@Override
190220
public void fine(String msg) {
191221
logTrace(Level.FINE, "unknown", msg);
192222
}
193223

194-
@Override
195224
public void fine(Supplier<String> msgSupplier) {
196225
logTrace(Level.FINE, "unknown", msgSupplier);
197226
}
227+
228+
public void warning(String format, Object... args) {
229+
if (!delegate.isLoggable(Level.WARNING)) {
230+
return;
231+
}
232+
String message = (args == null || args.length == 0) ? format : String.format(format, args);
233+
logWithCaller(Level.WARNING, null, message);
234+
}
235+
236+
public void severe(String msg, Throwable thrown) {
237+
logWithCaller(Level.SEVERE, thrown, msg);
238+
}
198239
}

0 commit comments

Comments
 (0)