gemini-code-assist[bot] commented on code in PR #38593:
URL: https://github.com/apache/beam/pull/38593#discussion_r3285428906
##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java:
##########
@@ -184,6 +185,7 @@ public boolean shouldRetry(IOException e) {
return RetryDeterminer.SOCKET_ERRORS.shouldRetry(e);
}
};
+ private static final AtomicBoolean overwriteLog = new AtomicBoolean(false);
Review Comment:
The `java.util.logging.LogManager` only maintains weak references to
loggers. If a logger is not held by a strong reference, it can be garbage
collected, and any configuration changes (such as the log level) will be lost.
To ensure the suppression persists, store the logger in a static field.
```suggestion
private static final java.util.logging.Logger GCS_IO_LOGGER =
java.util.logging.Logger.getLogger("com.google.cloud.hadoop.gcsio.GoogleCloudStorageImpl");
private static final AtomicBoolean overwriteLog = new AtomicBoolean(false);
```
##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java:
##########
@@ -726,9 +728,16 @@ public WritableByteChannel create(GcsPath path,
CreateOptions options) throws IO
}
}
+ @SuppressFBWarnings("LG_LOST_LOGGER_DUE_TO_WEAK_REFERENCE")
Review Comment:
This suppression is unnecessary if a strong reference to the logger is
maintained in a static field. It is better to address the underlying issue of
the weak reference than to suppress the warning.
##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java:
##########
@@ -726,9 +728,16 @@ public WritableByteChannel create(GcsPath path,
CreateOptions options) throws IO
}
}
+ @SuppressFBWarnings("LG_LOST_LOGGER_DUE_TO_WEAK_REFERENCE")
GoogleCloudStorage createGoogleCloudStorage(
GoogleCloudStorageOptions options, Storage storage, Credentials
credentials)
throws IOException {
+ // Suppress log spams in gcsio 3.0
+ if (overwriteLog.compareAndSet(false, true)) {
+
java.util.logging.Logger.getLogger("com.google.cloud.hadoop.gcsio.GoogleCloudStorageImpl")
+ .setLevel(java.util.logging.Level.SEVERE);
+ }
Review Comment:
Use the static logger field to set the log level. This ensures that the
`SEVERE` level setting is preserved even if the logger is internally recreated
by the `LogManager`.
```java
if (overwriteLog.compareAndSet(false, true)) {
GCS_IO_LOGGER.setLevel(java.util.logging.Level.SEVERE);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
