RFR: 7903580: Allow for re-attempting agent creation when an attempt fails [v3]

[Jaikiran Pai]

On Mon, 13 Nov 2023 18:39:05 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java line 1241:
>> 
>>> 1239:                     p.log("Re-attempting agent creation, attempt number " + i);
>>> 1240:                 }
>>> 1241:                 Agent agent = p.getAgent(absTestScratchDir().toFile(), jdk, vmOpts.toList(),
>> 
>> I initially wanted to do this re-attempt logic within the `Pool.getAgent(...)` method, but the way the pool is implemented, its methods are all `synchronized`. The `Agent` instance creation which happens in the `pool.getAgent(...)` involves socket connection attempt (which can wait for a long time). This effectively is a stop the world operation for jtreg process because none of the other methods on the pool, like returning back an agent from an already complete test (action) stalls till this method returns. The `synchronization` constructs in the pool isn't something that's introduced in this PR and instead are already present (and may need some improvement, but that's for a separate discussion/fix). Adding this re-attempt logic within the `pool.getAgent(...)` is going to extend that "stall the world" operation. So I decided to have this implemented outside of that method. Since anyway this retry logic is an internal detail, we can move this logic into the `Pool`, once/if we impr
 oves the synchronization constructs in it.
>
> Can you not make this blob of code be a static or unsynchronized method in `Agent.Pool`?
> 
> Approximately speaking, if you can do this code here, you should be able to do it there.

Hello Jon,
> Can you not make this blob of code be a static or unsynchronized method in Agent.Pool?

That's a good idea. It makes it much more cleaner and internal to the pool implementation itself. I've updated the PR to move this newly introduced code into a package private `Pool.getAgent(...)` method without any synchronization. This method does the necessary re-attempts of getting an agent. The previous existing `synchronized` `getAgent(...)` has just been renamed to `private synchronized doGetAgent(...)` - no other changes have been introduced in that method.

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/173#discussion_r1392280813