Copilot commented on code in PR #8415:
URL: https://github.com/apache/storm/pull/8415#discussion_r2876440823
##########
storm-client/src/jvm/org/apache/storm/metric/FileBasedEventLogger.java:
##########
@@ -109,16 +125,47 @@ public void prepare(Map<String, Object> conf, Map<String,
Object> arguments, Top
@Override
public void log(EventInfo event) {
try {
- //TODO: file rotation
- eventLogWriter.write(buildLogMessage(event));
+ String logMessage = buildLogMessage(event);
+ byte[] logBytes = logMessage.getBytes(StandardCharsets.UTF_8);
+ int writeLength = logBytes.length +
System.lineSeparator().length();
+
+ if (currentFileSize + writeLength > maxFileSize) {
+ rotateFiles();
+ }
+
+ eventLogWriter.write(logMessage);
eventLogWriter.newLine();
+ currentFileSize += writeLength;
dirty = true;
} catch (IOException ex) {
LOG.error("Error logging event {}", event, ex);
throw new RuntimeException(ex);
}
Review Comment:
`log()` can race with the scheduled flush task (and other concurrent `log()`
calls) during rotation: `rotateFiles()` closes/moves `eventLogWriter` while the
flush thread may call `eventLogWriter.flush()` and while `currentFileSize` is
being updated. This can cause intermittent IOExceptions/RuntimeExceptions and
corrupted rotation behavior. Introduce a shared lock (or make `log`, rotation,
and the flush runnable mutually exclusive) so write/flush/close/move are atomic
w.r.t. each other, and update `currentFileSize` under the same lock.
--
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]
