RFR: 8267952: async logging supports to dynamically change tags and decorators [v6]

[David Holmes]

Hi Xin,

On 23/06/2021 2:50 am, Xin Liu wrote:
>> Support dynamic reconfiguration for async logging. 2 unittests are provided.
>> The regression test discovers a race condition in LogTagSet::log() even with
>> synchronous logging. It's not MT-safe if context switch happens between the
>> creation of LogDecorations  and LogOutputList::Iterator. fixed.
> 
> Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
> 
>   - AsyncLogWrite::flush() uses a synchronization token and wait for completion.
>     
>     This patch serializes all flush() calls in one thread.

I put this through our test system and tiers 6 and 7 also passed this way.

So to walk through the fix ...

When changing the configuration:
- first update the values so that future log entries will use the new 
ones (this includes the RCU check for "no readers")
- insert a token item into the enqueued messages for the AsyncLog thread 
to see
- wait on the flush_sem

In this way when the asyncLog thread finds the token it can signal the 
thread blocked in the flush() call and that thread can then proceed to 
change the configuration knowing no enqueued messages can be dependent 
on anything being changed.

I think that seem okay. Only ever doing the writing in the asyncLog 
thread seems cleaner too.

The only thing I need to double-check is whether there is any potential 
termination race.

In the meantime, I would suggest closing this PR and opening a new one 
for JDK 17 as we need to get this fixed there ASAP. And a fresh PR will 
make things a lot clearer for other reviewers.

Thanks,
David

>   - Merge branch 'master' into JDK-8267952
>   - Revert "Use Atomic::load/store for decorators."
>     
>     This reverts commit f279f2639bc638a35ae6c716987feb22d36a2a07.
>   - Revert "Fix VC++ warning C4099"
>     
>     This reverts commit 157a065876850df784d9e7c034e106f4532b607a.
>   - Fix VC++ warning C4099
>   - Use Atomic::load/store for decorators.
>     
>     This patch also installed a sanity check which will assert that
>     LogOutput::_decorators is subset of LogDecocrations. It will disclose
>     more information if the assertion fails.
>   - Merge branch 'master' into JDK-8267952
>   - Add comments in LogTagSet::log().
>   - 8267952: async logging supports to dynamically change tags and decorators
> 
> -------------
> 
> Changes:
>    - all: https://git.openjdk.java.net/jdk/pull/4408/files
>    - new: https://git.openjdk.java.net/jdk/pull/4408/files/157a0658..ae3ae36d
> 
> Webrevs:
>   - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4408&range=05
>   - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4408&range=04-05
> 
>    Stats: 18757 lines in 334 files changed: 11361 ins; 6472 del; 924 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/4408.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/4408/head:pull/4408
> 
> PR: https://git.openjdk.java.net/jdk/pull/4408
>