sharanggupta opened a new pull request, #4398: URL: https://github.com/apache/logging-log4j2/pull/4398
Fixes #4362. `RollingFileManager` rounds the time of an existing file to the nearest second (#3068, #3872), but `OnStartupTriggeringPolicy.initialize` compared it with the JVM start time in milliseconds. A file created by this JVM in the first half of its start second therefore looked older than the JVM, and the policy rolled it as soon as a second `LoggerContext` re-initialized the policy for the same file. I kept the rounding, which exists for the ext4 timestamp caching of #3068, and made the comparison symmetric: both the file time and the start time go through `RollingFileManager.alignMillisToSecond` before they are compared. Rounding both sides, rather than only the start time, matters because the manager also sets an *unrounded* `initialTime = System.currentTimeMillis()` after a rollover. Rounding is monotonic, so a file whose time is at or after the JVM start never compares as older, whichever of the two kinds of value it carries. Trade-off worth a reviewer's eye: a file created by an *earlier* process less than about a second before this JVM started now compares as equal and is not rolled at startup. That is the one-second precision that the rounding already introduced, and it fails safe (no rollover, no lost data). Telling those files apart would need sub-second file timestamps, which the rounding deliberately stops relying on. To make the start time testable I added a package-private constructor `OnStartupTriggeringPolicy(long minSize, long startTime)`; the plugin factory still uses `JVM_START_TIME`. ## Tests `OnStartupTriggeringPolicyTest.testPolicyDoesNotRolloverFileCreatedByThisJvm` is parameterized over `(startTime, fileTime, expectedRollover)` and drives `initialize` with a mocked `RollingFileManager`: * File created by this JVM in the first half of its start second, rounded down by the manager: no rollover. **This is the case of the issue and the only row that fails on `2.x`.** * File created right after a rollover (unrounded time, start time rounds up): no rollover. Guards against rounding only the start time. * File created exactly at the start: no rollover. * Two files created by an earlier process: rolled over. Guards against suppressing legitimate rollovers. ## Verification * The new test and the whole `org.apache.logging.log4j.core.appender.rolling` package of `log4j-core-test` pass (289 tests, 0 failures, run through `verify` including spotless). A first run had one failure in the unrelated, sleep-based `RollingAppenderSizeCompressPermissionsTest` (its configuration has no `OnStartupTriggeringPolicy`); it passes on the baseline when run on its own and did not recur on a rerun. * I did not run the multi-JVM reproducer from the issue, only the unit test with its timestamps, and I did not run the complete `./mvnw verify` of the whole reactor locally; I rely on CI for it. Generative-AI disclosure (ASF guidance): Claude Code (Anthropic) assisted in drafting the change, tests and this description; the result was reviewed, the new test was confirmed to fail on `2.x` and pass with the change, and the tests and formatting checks above were run before opening this PR. ## Checklist * [x] Base your changes on `2.x` branch if you are targeting Log4j 2; use `main` otherwise * [ ] `./mvnw verify` succeeds: not run for the whole reactor locally, see Verification above * [x] Non-trivial changes contain an entry file in the `src/changelog/.2.x.x` directory * [x] Tests are provided (`OnStartupTriggeringPolicyTest`: 1 new parameterized test, 5 cases) -- 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]
