On Fri, 22 Aug 2025 17:18:40 GMT, Phil Race <p...@openjdk.org> wrote:

> > @prrace the change maintains the same absolute timeout value for those 
> > tests. Before the default of 120 was multiplied by the timeoutFactor of 4 
> > to given 480. Now the value 480 is multiplied by the timeoutFactor of 1 to 
> > give 480. And IIRC Leo only did that for tests that demonstrated a timeout 
> > with the new default settings (120*1). It is not practical for Leo to 
> > investigate every changed test to see if it could get away with a value 
> > between 120 and 480. The change just maintains the status quo. Test owners 
> > are free to investigate further if they think it worth fine tuning these 
> > values.
> 
> I don't agree. If you are going to modify individual tests, you need to 
> demonstrate what you did for that test is justified or don't do it.
> 
> I am also questioning whether such a time out was demonstrated for this test. 
> I've searched the entire history of CI jobs and I don't see where Leo had 
> such a timeout of this test. I can send you my query off-line so you can 
> check it. Maybe it is incomplete.

I will revert this change. Thanks for finding it. I think the timeout was found 
in a local run with a timeout factor of 0.5 and a local fix to 
"CODETOOLS-7903937: JTREG uses timeout factor on socket timeout but not on 
KEEPALIVE". CI history tells me that the run time of the test is stable and 
that we have a margin.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26749#discussion_r2307351823

Reply via email to