RFR: 8330814: Cleanups for KeepAliveCache tests [v2]

Daniel Fuchs dfuchs at openjdk.org
Tue Apr 30 15:07:06 UTC 2024


On Thu, 25 Apr 2024 08:04:16 GMT, Christoph Langer <clanger at openjdk.org> wrote:

>> test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 154:
>> 
>>> 152: 
>>> 153:                     // if Keep-Alive-SocketCleaner consumes more than 50% of cpu then we
>>> 154:                     // can assume a recursive loop.
>> 
>> Interesting test case. I'm a bit surprised we haven't seen this intermittently fail. Speaking of which, I see that the change to this test proposes to remove it from `/othervm` and potentially run it in agent vm mode. Given the kind of checks this test is doing, I think it's better to let it run as a `othervm` test to keep the test as isolated as possible.
>
> I've thought about this, too. However, I see the only critical point why it could merit a `/othervm` test is this thing with querying the thread CPU time of the Keep-Alive-SocketCleaner thread. But I think the likelihood of this failing is the same within standard test and `/othervm`. So I'd prefer to change to standard test, since this is one of the improvements of this change.

The cache is global. Who knows what it might already contain when the test starts and what may be left behind when the test ends. So I agree that it may be prudent to keep `/othervm`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1584992784


More information about the net-dev mailing list