On Thu, 13 Feb 2025 09:32:29 GMT, David Beaumont <d...@openjdk.org> wrote:
>> hmmm... Thanks for chiming in Jason. Good point. So what can happen here is >> that if multiple threads log concurrently to the same FileHandler then log >> records might continue to get written to the file after the limit has been >> reached but before the check that would rotate is performed. In pathological >> cases where new records continue to arrive and manage to enter the monitor >> before the post-publish action that rotate can do so, this could become >> problematic. >> >> I think what you are proposing would work, since we control the >> implementation of super.publish(). > > We could, but I don't think it matters. This issue is the first one pointed > out in the CSR, and my current feeling is that since it's stated that the > limit is "best effort" and there's always the chance that the final log > before rotate was massive anyway, and the risk of this sort of interleaving > is low, it's not going to make things qualitatively worse than they are now. > If the docs promised that file wouldn't grow beyond the limit, I'd definitely > have done things something like you're suggesting. To be clear I'm with you on your approach and hold much of the same views. >We could, but I don't think it matters. I'm just focusing on this aspect because it is usually the thing comes back to bite me. I think it matters when resources are limited (diskspace) and file size limits are low. Write a test with filehander set to 2 files at 1M in size. 2. Create a custom Formatter that uses a count down latch set to 10 and returns the log sequence as a string. Install it on the filehander. 3. Start 10 threads that log ~1M of data. 4. Assert that only 2 log sequences are seen when scanning both files. Old code should only allow at most 2 records which helps control the overshoot of file size. For context, I've built and have used custom Formatters that would have blocking behavior like the count down latch test. This is not a strawman test. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1955111379
