Request for review: 6941923: RFE: Handling large log files produced by long running Java Applications

[Y. S. Ramakrishna]

Hi Yumin --

On 05/05/11 08:38, Yumin Qi wrote:
...
> So, default behavior not changed if GCLogFileSize=0 and 
> NumberOfGCLogFiles=0.
> If #file > 0, but GCLogFileSize=0, it is still confused since we use 
> file.0 and output to it at no limit. The #file seems only make sense for 1.

Well, I was using a limit ordinal analogy here. When you reach the
first limit ordinal, \omega, you will flip to the next file.
Of course in real life we won't. But it's OK to do what you
suggest below. It just entails extra args checking, which is fine too.

> 
> I would like if #file > 0 and GCLogFileSize=0, we still keep current 
> behavior - no rotation. (GCLogFileSize default is 0, no limit as you said).

That's fine too.

> 
> That is, if rotation stands, UseGCLogRotation ,must be true.
> 1) If GCLogFileSize is default (0), no rotation; UseGCLogFileRotation is 
> false
> 2) If #file is default (0), no rotation.   UseGCLogFileRotation is false.

OK; i am sure you will need to straighten this out in the
CCC spec and spell out the restrictions if any on
the allowed combinations etc.

> This is why I need your review here. The rotation only done at STW, so 
> seems the lock is not necessary here. Remove the locking code, will put 
> comments to indicating this function only called at safepoint and add 
> assert at safepoint.

It's fine to do the locking. I have no objection to the locking;
my question was why the locking was elided for those two cases.
It would seem to me that you'd need some way to make sure that
the stream is not under active use when the rotation occurs,
and the lock would be a fine way to ensure that the stream is
never accessed except under protection of the lock. I can see
that the rotation is done at a STW pause when no mutator is
running. It turns out today that no daemons run at that time
either, but it is conceivable that some may in the future
and have access to or attempt tp access that stream/log.
We should make the code robust in the face of future such
evolution, by at least putting in enough controls, and
the lock would seem to me to be a fine mechanism to do so.
(After auditing the code to make sure that the handle is
not accessible outside the lock.)

-- ramki