Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

On Thu, Sep 12, 2019 at 10:53 AM Pavel Rappo  wrote:

> Martin,
>
> May I add you to the list of people who reviewed this change? I mean the
> concurrency-related portion of the change, specifically in the test. Not
> the LDAP part.
>

Sure!

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

Martin, 

May I add you to the list of people who reviewed this change? I mean the 
concurrency-related portion of the change, specifically in the test. Not the 
LDAP part.

> On 12 Sep 2019, at 16:21, Martin Buchholz  wrote:
> 
> 
> 
> On Thu, Sep 12, 2019 at 4:36 AM Pavel Rappo  wrote:
> 
> I should have expressed myself more clear. I meant that the main thread and 
> the thread created (and started) by `newStartedThread` (the test tread) do 
> not wait for *each other* before they begin. This might be needed to make 
> sure that they start at the same time (loosely speaking, of course). One 
> might argue that the importance of this diminishes with the increase of the 
> timeout that the main thread uses to wait for the test thread to die and the 
> accuracy of measurements one agrees to tolerate.
> 
> 
> You want coordination?  We got coordination.
> 
> Here's some infrastructure that uses a threadsStarted latch:
> 
> void assertThrowInterruptedExceptionWhenInterrupted(Action[] actions) {
> int n = actions.length;
> Future[] futures = new Future[n];
> CountDownLatch threadsStarted = new CountDownLatch(n);
> CountDownLatch done = new CountDownLatch(n);
> 
> for (int i = 0; i < n; i++) {
> Action action = actions[i];
> futures[i] = cachedThreadPool.submit(new CheckedRunnable() {
> public void realRun() throws Throwable {
> threadsStarted.countDown();
> try {
> action.run();
> shouldThrow();
> }
> catch (InterruptedException success) {}
> catch (Throwable fail) { threadUnexpectedException(fail); 
> }
> assertFalse(Thread.interrupted());
> done.countDown();
> }});
> }
> 
> await(threadsStarted);
> assertEquals(n, done.getCount());
> for (Future future : futures) // Interrupt all the tasks
> future.cancel(true);
> await(done);
> }
> 
> When communicating a request, just like in real life, you can't force others 
> to do anything, so be politely Canadian and use "please" in the name of your 
> latch
> 
> /**
>  * await throws InterruptedException if interrupted before counted down
>  */
> public void testAwait_Interruptible() {
> final CountDownLatch l = new CountDownLatch(1);
> final CountDownLatch pleaseInterrupt = new CountDownLatch(1);
> Thread t = newStartedThread(new CheckedRunnable() {
> public void realRun() throws InterruptedException {
> Thread.currentThread().interrupt();
> try {
> l.await();
> shouldThrow();
> } catch (InterruptedException success) {}
> assertFalse(Thread.interrupted());
> 
> pleaseInterrupt.countDown();
> try {
> l.await();
> shouldThrow();
> } catch (InterruptedException success) {}
> assertFalse(Thread.interrupted());
> 
> assertEquals(1, l.getCount());
> }});
> 
> await(pleaseInterrupt);
> if (randomBoolean()) assertThreadBlocks(t, Thread.State.WAITING);
> t.interrupt();
> awaitTermination(t);
> }
>  

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Daniel Fuchs]

Hi Pavel,

The changes in webrev.02 look good to me as well.
Thanks for renaming the constants in the test (I mean
TOLERANCE vs RIGHT_MARGIN and LEFT_MARGIN)  - that makes
for a much better read.

best regards,

-- daniel

On 12/09/2019 13:26, Rob McKenna wrote:

Here's the updated version of the RFR based on the discussion so far:

 http://cr.openjdk.java.net/~prappo/8151678/webrev.02/

For the reviewers. I totally forgot to explain why there's a multiplier of 2 in 
some of the timeout calculations. The reason is the current behavior of 
InitialDirContext. The supplied connect timeout seems to be used twice. Once 
for making the actual TCP connection [1] and the second time while waiting for 
the server to respond to the BIND message [2]. Thus, the total time spent in 
that InitialDirContext ctor may be twice the expected. I believe it's a bug, 
but the bug that is not related to the issue in question. The current issue 
(8151678) is about intermittent failures of LdapTimeoutTest.

-Pavel

---
[1]http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l296,http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l320
[2]http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l154,http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l365

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

On Thu, Sep 12, 2019 at 4:36 AM Pavel Rappo  wrote:

>
> I should have expressed myself more clear. I meant that the main thread
> and the thread created (and started) by `newStartedThread` (the test tread)
> do not wait for *each other* before they begin. This might be needed to
> make sure that they start at the same time (loosely speaking, of course).
> One might argue that the importance of this diminishes with the increase of
> the timeout that the main thread uses to wait for the test thread to die
> and the accuracy of measurements one agrees to tolerate.
>
>
You want coordination?  We got coordination.

Here's some infrastructure that uses a threadsStarted latch:

void assertThrowInterruptedExceptionWhenInterrupted(Action[] actions) {
int n = actions.length;
Future[] futures = new Future[n];
CountDownLatch threadsStarted = new CountDownLatch(n);
CountDownLatch done = new CountDownLatch(n);

for (int i = 0; i < n; i++) {
Action action = actions[i];
futures[i] = cachedThreadPool.submit(new CheckedRunnable() {
public void realRun() throws Throwable {
threadsStarted.countDown();
try {
action.run();
shouldThrow();
}
catch (InterruptedException success) {}
catch (Throwable fail) {
threadUnexpectedException(fail); }
assertFalse(Thread.interrupted());
done.countDown();
}});
}

await(threadsStarted);
assertEquals(n, done.getCount());
for (Future future : futures) // Interrupt all the tasks
future.cancel(true);
await(done);
}

When communicating a request, just like in real life, you can't force
others to do anything, so be politely Canadian and use "please" in the name
of your latch

/**
 * await throws InterruptedException if interrupted before counted down
 */
public void testAwait_Interruptible() {
final CountDownLatch l = new CountDownLatch(1);
final CountDownLatch pleaseInterrupt = new CountDownLatch(1);
Thread t = newStartedThread(new CheckedRunnable() {
public void realRun() throws InterruptedException {
Thread.currentThread().interrupt();
try {
l.await();
shouldThrow();
} catch (InterruptedException success) {}
assertFalse(Thread.interrupted());

pleaseInterrupt.countDown();
try {
l.await();
shouldThrow();
} catch (InterruptedException success) {}
assertFalse(Thread.interrupted());

assertEquals(1, l.getCount());
}});

await(pleaseInterrupt);
if (randomBoolean()) assertThreadBlocks(t, Thread.State.WAITING);
t.interrupt();
awaitTermination(t);
}

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Rob McKenna]

This looks great to me. Thanks for taking a look at it.

-Rob

On 12/09/19 12:36, Pavel Rappo wrote:
> > On 12 Sep 2019, at 03:54, Martin Buchholz  wrote:
> > 
> > We're mostly in agreement.
> 
> Good to hear and I agree!
> 
> > On Wed, Sep 11, 2019 at 11:02 AM Pavel Rappo  > > wrote:
> > 
> >  
> > a. is not using any synchronization aid to make sure both threads wait for 
> > each other (probably, the timeout value accommodates that)
> > 
> > Thread.join is a synchronization aid!  But there's no shortage of others.  
> > We typically use a CountDownLatch to synchronize with another thread.
> 
> 
> I should have expressed myself more clear. I meant that the main thread and 
> the thread created (and started) by `newStartedThread` (the test tread) do 
> not wait for *each other* before they begin. This might be needed to make 
> sure that they start at the same time (loosely speaking, of course). One 
> might argue that the importance of this diminishes with the increase of the 
> timeout that the main thread uses to wait for the test thread to die and the 
> accuracy of measurements one agrees to tolerate.
> 
> I'm seeing differences in our timeout-measuring routines as just a difference 
> in tastes. Since a particular behavior here is not guaranteed anyway (as it's 
> not a hard real-time system), all we can hope for in both cases is adequate 
> timeout values and useful diagnostic output.
> 
> Thanks for patiently staying with this thread for so long, Martin.
> 
> 
>  ***
> 
> 
> Here's the updated version of the RFR based on the discussion so far:
> 
> http://cr.openjdk.java.net/~prappo/8151678/webrev.02/
> 
> For the reviewers. I totally forgot to explain why there's a multiplier of 2 
> in some of the timeout calculations. The reason is the current behavior of 
> InitialDirContext. The supplied connect timeout seems to be used twice. Once 
> for making the actual TCP connection [1] and the second time while waiting 
> for the server to respond to the BIND message [2]. Thus, the total time spent 
> in that InitialDirContext ctor may be twice the expected. I believe it's a 
> bug, but the bug that is not related to the issue in question. The current 
> issue (8151678) is about intermittent failures of LdapTimeoutTest.
> 
> -Pavel
> 
> ---
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l296,
>  
> http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l320
> [2] 
> http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l154,
>  
> http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l365
> 

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

> On 12 Sep 2019, at 03:54, Martin Buchholz  wrote:
> 
> We're mostly in agreement.

Good to hear and I agree!

> On Wed, Sep 11, 2019 at 11:02 AM Pavel Rappo  > wrote:
> 
>  
> a. is not using any synchronization aid to make sure both threads wait for 
> each other (probably, the timeout value accommodates that)
> 
> Thread.join is a synchronization aid!  But there's no shortage of others.  We 
> typically use a CountDownLatch to synchronize with another thread.


I should have expressed myself more clear. I meant that the main thread and the 
thread created (and started) by `newStartedThread` (the test tread) do not wait 
for *each other* before they begin. This might be needed to make sure that they 
start at the same time (loosely speaking, of course). One might argue that the 
importance of this diminishes with the increase of the timeout that the main 
thread uses to wait for the test thread to die and the accuracy of measurements 
one agrees to tolerate.

I'm seeing differences in our timeout-measuring routines as just a difference 
in tastes. Since a particular behavior here is not guaranteed anyway (as it's 
not a hard real-time system), all we can hope for in both cases is adequate 
timeout values and useful diagnostic output.

Thanks for patiently staying with this thread for so long, Martin.


 ***


Here's the updated version of the RFR based on the discussion so far:

http://cr.openjdk.java.net/~prappo/8151678/webrev.02/

For the reviewers. I totally forgot to explain why there's a multiplier of 2 in 
some of the timeout calculations. The reason is the current behavior of 
InitialDirContext. The supplied connect timeout seems to be used twice. Once 
for making the actual TCP connection [1] and the second time while waiting for 
the server to respond to the BIND message [2]. Thus, the total time spent in 
that InitialDirContext ctor may be twice the expected. I believe it's a bug, 
but the bug that is not related to the issue in question. The current issue 
(8151678) is about intermittent failures of LdapTimeoutTest.

-Pavel

---
[1] 
http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l296,
 
http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l320
[2] 
http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l154,
 
http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l365

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

We're mostly in agreement.
(Also, I'm not actually an ldap reviewer.)

On Wed, Sep 11, 2019 at 11:02 AM Pavel Rappo  wrote:

> I agree that this example [1] looks readable (depending on a reader, of
> course). I think it looks readable mostly because it is very explicit.
> However, in a domain other than concurrency we are not usually concerned
> with this level of detail.


I think you *are* in the domain of concurrency!


> a. is not using any synchronization aid to make sure both threads wait for
> each other (probably, the timeout value accommodates that)
>

Thread.join is a synchronization aid!  But there's no shortage of others.
We typically use a CountDownLatch to synchronize with another thread.


> b. uses a number of tiny low-level methods like newStartedThread,
> awaitTermination, millisElapsedSince, manual time assertions, etc.
> c. has assertions spread across 2 threads
>

I don't see that as a problem, as long as every assertion failure
propagates properly to become a test failure.


>
> (b) probably allows you to reuse components in places unrelated to
> timeouts. At the same time you don't seem to have any higher-level reusable
> component (i.e. my guess is that this code is more or less repeated in
> other places in that test suite, which is not necessarily a bad thing).
>
> However, I fully agree with you that this functionality should be a
> utility method of the test library.
>
> -
> [1] I assume this is an excerpt from CountDownLatchTest.java. If so, then
> for the reader's convenience this method could be seen in its context at
> http://hg.openjdk.java.net/jdk/jdk/file/d52f77f0acb5/test/jdk/java/util/concurrent/tck/CountDownLatchTest.java#l198
> [2] I'm not saying that those things are wrong or that I disagree with any
> of that. It's just an observation from reading this example.
>
>

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

Martin,

> On 10 Sep 2019, at 17:40, Martin Buchholz  wrote:
> 
> Here's a canonical example of an "expected timeout" test that seems natural 
> and readable to me.
> timeoutMillis() returns 12ms, adjusted by timeout factor.  Ldap might need a 
> bigger value.
> 
> 
> /**
>  * timed await times out if not counted down before timeout
>  */
> public void testAwaitTimeout() throws InterruptedException {
> final CountDownLatch l = new CountDownLatch(1);
> Thread t = newStartedThread(new CheckedRunnable() {
> public void realRun() throws InterruptedException {
> assertEquals(1, l.getCount());
> 
> long startTime = System.nanoTime();
> assertFalse(l.await(timeoutMillis(), MILLISECONDS));
> assertTrue(millisElapsedSince(startTime) >= timeoutMillis());
> 
> assertEquals(1, l.getCount());
> }});
> 
> awaitTermination(t);
> assertEquals(1, l.getCount());
> }


I agree that this example [1] looks readable (depending on a reader, of 
course). I think it looks readable mostly because it is very explicit. However, 
in a domain other than concurrency we are not usually concerned with this level 
of detail. In such cases I would prefer to have a small set of focused methods 
that would allow me to explain my intentions succinctly and only once, taking 
care of all the low-level details.

As for the internal mechanics, I can see that this example [2]:

a. is not using any synchronization aid to make sure both threads wait for each 
other (probably, the timeout value accommodates that)
b. uses a number of tiny low-level methods like newStartedThread, 
awaitTermination, millisElapsedSince, manual time assertions, etc.
c. has assertions spread across 2 threads

(b) probably allows you to reuse components in places unrelated to timeouts. At 
the same time you don't seem to have any higher-level reusable component (i.e. 
my guess is that this code is more or less repeated in other places in that 
test suite, which is not necessarily a bad thing).

However, I fully agree with you that this functionality should be a utility 
method of the test library.

-
[1] I assume this is an excerpt from CountDownLatchTest.java. If so, then for 
the reader's convenience this method could be seen in its context at 
http://hg.openjdk.java.net/jdk/jdk/file/d52f77f0acb5/test/jdk/java/util/concurrent/tck/CountDownLatchTest.java#l198
[2] I'm not saying that those things are wrong or that I disagree with any of 
that. It's just an observation from reading this example.

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

Here's a canonical example of an "expected timeout" test that seems natural
and readable to me.
timeoutMillis() returns 12ms, adjusted by timeout factor.  Ldap might need
a bigger value.


/**
 * timed await times out if not counted down before timeout
 */
public void testAwaitTimeout() throws InterruptedException {
final CountDownLatch l = new CountDownLatch(1);
Thread t = newStartedThread(new CheckedRunnable() {
public void realRun() throws InterruptedException {
assertEquals(1, l.getCount());

long startTime = System.nanoTime();
assertFalse(l.await(timeoutMillis(), MILLISECONDS));
assertTrue(millisElapsedSince(startTime) >=
timeoutMillis());

assertEquals(1, l.getCount());
}});

awaitTermination(t);
assertEquals(1, l.getCount());
}

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

On Tue, Sep 10, 2019 at 7:25 AM Pavel Rappo  wrote:

> > On 6 Sep 2019, at 20:02, Martin Buchholz  wrote:
> >
> > Martin's rules for test methods:
> > - never have more than 100 millisecond total expected runtime
> > - never have more than 12 millisecond blocking time for any single
> operation
> > - yet be able to survive a one-time 5 second thread suspension at any
> time
>
> No doubt these are reputable numbers however magic they seem. I expect
> these make sense in a highly focused code, which j.u.c might be an example
> of. LDAP, on the other hand, does everything but the kitchen sink. So much
> so that I cannot guarantee any of those metrics to hold.
>
> The latter one is of particular interest to me. Could you maybe elaborate
> on what "a rare 5-second thread suspension" is? (Not that it will make me
> change the code, but out of pure curiosity.)
>

Suppose you are waiting for some other thread to complete some trivial
observable operation, like counting down a latch.  At some point you want
to time out and report failure.  How long before spurious failures
disappear entirely?  We used to use 250ms and see rare failures - too
small!  After switching to 10 seconds (scaled by timeout factor) spurious
failures due to thread suspension have disappeared in practice.  The
longest thread suspension I ever actually observed was 4 seconds.  Of
course,  no guarantees - higher priority processes can always hog the cpu.

If you are waiting for a less trivial operation, you may need a higher
value than our magic 10 seconds.

An alternative is to wait "forever" and rely on jtreg's own timeout handler
to kick in and fail the test for you.

public void await(Semaphore semaphore) {
boolean timedOut = false;
try {
timedOut = !semaphore.tryAcquire(LONG_DELAY_MS, MILLISECONDS);
} catch (Throwable fail) {
threadUnexpectedException(fail);
}
if (timedOut)
fail("timed out waiting for Semaphore for "
 + (LONG_DELAY_MS/1000) + " sec");
}

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

> On 6 Sep 2019, at 20:02, Martin Buchholz  wrote:
> 
> Martin's rules for test methods:
> - never have more than 100 millisecond total expected runtime
> - never have more than 12 millisecond blocking time for any single operation
> - yet be able to survive a one-time 5 second thread suspension at any time

No doubt these are reputable numbers however magic they seem. I expect these 
make sense in a highly focused code, which j.u.c might be an example of. LDAP, 
on the other hand, does everything but the kitchen sink. So much so that I 
cannot guarantee any of those metrics to hold.

The latter one is of particular interest to me. Could you maybe elaborate on 
what "a rare 5-second thread suspension" is? (Not that it will make me change 
the code, but out of pure curiosity.)

> adjusting wait time looks like an anti-pattern to me.  You can't stop a rare 
> 5-second thread suspension at line 387.
> 
>  385 final long waitTime = hiMillis -
>  386 NANOSECONDS.toMillis(System.nanoTime() - startNanos); // 
> (2) adjust wait time
>  387 
>  388 try {
>  389 T r = task.get(waitTime, MILLISECONDS); // (3) wait for the 
> task to complete
> 

I agree it might make the code look more complicated, but it doesn't deserve 
the name "anti-pattern". Is it a bad solution? Should I expect it to backfire? 
The reason for using that is to make sure the code accommodates for a given 
timeout once, not many times.

Yes, I agree I can't stop a thread suspension at line 387 or anywhere else for 
that matter, but I shouldn't account for it either. Maybe just once -- in the 
value of the timeout.

Again, many thanks for looking at it!

Thoughts?

> On Fri, Sep 6, 2019 at 9:33 AM Pavel Rappo  wrote:
> > On 6 Sep 2019, at 15:59, Martin Buchholz  wrote:
> > 
> > I took another look at LdapTimeoutTest.java.
> 
> Many thanks!
> 
> > I was surprised to see RIGHT_MARGIN.  In jsr166 we succeed in having one 
> > timeout that is long enough to "never happen".  I'm still advocating the 10 
> > second value.
> > 
> > I was surprised to see LEFT_MARGIN.  We just fixed Thread.sleep, so we have 
> > no known problems with JDK methods returning early - you can trust timed 
> > get!
> > You start measuring, by calling nanoTime, before you start the activity you 
> > are measuring, so there should be no need for LEFT_MARGIN.
> 
> You raised many good points. Let me try to address them.
> 
> 1. RIGHT_MARGIN is not used for checking that the activity has stuck 
> infinitely (assertIncompletion). INFINITY_MILLIS is used for that. 
> RIGHT_MARGIN is used for checking that the activity takes some predefined 
> amount of time (roughly).
> 
> 2. As for the numeric value of INFINITY_MILLIS, the reason I chose 20 seconds 
> is twofold. Firstly, the code under test is subject to different timeouts. 
> Every timeout value should be distinctive. Otherwise, how would I 
> differentiate between them? For example, if I chose INFINITY_MILLIS to be 10 
> seconds how would I know if the code is stuck due to the read timeout of 10 
> seconds or the "infinite timeout"? Secondly, I must take into account slow 
> machines. Really slow virtual machines. Hence, minimal timeouts 
> (read/connect) have a magnitude of seconds and tens of seconds and the 
> "rightmost", infinite timeout, is 20 seconds.
> 
> 3. LEFT_MARGIN might no longer be needed due to the fact that no timed 
> methods return early (actually there is a comment about it inside those two 
> assert methods).
> 
> > We have some fresh thread-awaiting code here:
> > http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?view=markup#l1443
> 
> Interesting.
> 
> > Instead of communicating startTime from the test thread back to the main 
> > thread, I would do my loMillis checking in the test thread, and hiMillis 
> > checking in the main thread, like e.g. compare with a fresh test method 
> > testTimedOffer
> > http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/ArrayBlockingQueueTest.java?view=markup#l394
> 
> Understood. However, that might be a matter of taste. I prefer to have all 
> the calculations and error handling in one place. Unless there's a good 
> reason I wouldn't change it.
> 
> > Timeouts should be adjusted via Utils.adjustTimeout
> 
> That makes perfect sense. I never knew this method existed. Thanks!
> 
> > On Fri, Sep 6, 2019 at 4:31 AM Pavel Rappo  wrote:
> > Martin, thanks for having a look at it.
> > 
> > I'd appreciate if you could have a look at the timeout measuring mechanics 
> > in assertCompletion/assertIncompletion specifically, maybe to spot 
> > something that is grossly inadequate.
> > 
> > I tried to accommodate some usual suspects of timeout measurements 
> > failures. I understand that since we're not working with real-time systems, 
> > my attempts to build bullet-proof measurement mechanics are futile.
> > 
> > -Pavel
> > 
> > > On 30 Aug 2019, at 18:19, Martin Buchholz  wrote:
> > > 
> > 

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

Martin's rules for test methods:
- never have more than 100 millisecond total expected runtime
- never have more than 12 millisecond blocking time for any single operation
- yet be able to survive a one-time 5 second thread suspension at any time

---

adjusting wait time looks like an anti-pattern to me.  You can't stop a
rare 5-second thread suspension at line 387.

 385 final long waitTime = hiMillis -
 386 NANOSECONDS.toMillis(System.nanoTime() - startNanos);
// (2) adjust wait time
 387
 388 try {
 389 T r = task.get(waitTime, MILLISECONDS); // (3) wait for
the task to complete


On Fri, Sep 6, 2019 at 9:33 AM Pavel Rappo  wrote:

> > On 6 Sep 2019, at 15:59, Martin Buchholz  wrote:
> >
> > I took another look at LdapTimeoutTest.java.
>
> Many thanks!
>
> > I was surprised to see RIGHT_MARGIN.  In jsr166 we succeed in having one
> timeout that is long enough to "never happen".  I'm still advocating the 10
> second value.
> >
> > I was surprised to see LEFT_MARGIN.  We just fixed Thread.sleep, so we
> have no known problems with JDK methods returning early - you can trust
> timed get!
> > You start measuring, by calling nanoTime, before you start the activity
> you are measuring, so there should be no need for LEFT_MARGIN.
>
> You raised many good points. Let me try to address them.
>
> 1. RIGHT_MARGIN is not used for checking that the activity has stuck
> infinitely (assertIncompletion). INFINITY_MILLIS is used for that.
> RIGHT_MARGIN is used for checking that the activity takes some predefined
> amount of time (roughly).
>
> 2. As for the numeric value of INFINITY_MILLIS, the reason I chose 20
> seconds is twofold. Firstly, the code under test is subject to different
> timeouts. Every timeout value should be distinctive. Otherwise, how would I
> differentiate between them? For example, if I chose INFINITY_MILLIS to be
> 10 seconds how would I know if the code is stuck due to the read timeout of
> 10 seconds or the "infinite timeout"? Secondly, I must take into account
> slow machines. Really slow virtual machines. Hence, minimal timeouts
> (read/connect) have a magnitude of seconds and tens of seconds and the
> "rightmost", infinite timeout, is 20 seconds.
>
> 3. LEFT_MARGIN might no longer be needed due to the fact that no timed
> methods return early (actually there is a comment about it inside those two
> assert methods).
>
> > We have some fresh thread-awaiting code here:
> >
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?view=markup#l1443
>
> Interesting.
>
> > Instead of communicating startTime from the test thread back to the main
> thread, I would do my loMillis checking in the test thread, and hiMillis
> checking in the main thread, like e.g. compare with a fresh test method
> testTimedOffer
> >
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/ArrayBlockingQueueTest.java?view=markup#l394
>
> Understood. However, that might be a matter of taste. I prefer to have all
> the calculations and error handling in one place. Unless there's a good
> reason I wouldn't change it.
>
> > Timeouts should be adjusted via Utils.adjustTimeout
>
> That makes perfect sense. I never knew this method existed. Thanks!
>
> > On Fri, Sep 6, 2019 at 4:31 AM Pavel Rappo 
> wrote:
> > Martin, thanks for having a look at it.
> >
> > I'd appreciate if you could have a look at the timeout measuring
> mechanics in assertCompletion/assertIncompletion specifically, maybe to
> spot something that is grossly inadequate.
> >
> > I tried to accommodate some usual suspects of timeout measurements
> failures. I understand that since we're not working with real-time systems,
> my attempts to build bullet-proof measurement mechanics are futile.
> >
> > -Pavel
> >
> > > On 30 Aug 2019, at 18:19, Martin Buchholz  wrote:
> > >
> > > Not really a review, but:
> > >
> > > For many years we've been using 10 seconds (scaled by timeout factor)
> as a duration long enough that a timeout is a real failure.
> > > Which is close to your own 20 seconds.  Of course, no value is surely
> safe.
> > >
> > > Probably, parallel testing infrastructure for timeouts should be a
> test library method.  I do something similar in JSR166TestCase
> > >
> > > /**
> > >  * Runs all the given actions in parallel, failing if any fail.
> > >  * Useful for running multiple variants of tests that are
> > >  * necessarily individually slow because they must block.
> > >  */
> > > void testInParallel(Action ... actions) {
> > > ExecutorService pool = Executors.newCachedThreadPool();
> > > try (PoolCleaner cleaner = cleaner(pool)) {
> > >
> > > On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs 
> wrote:
> > > On 30/08/2019 13:54, Pavel Rappo wrote:
> > > > Updated,
> > > >
> > > >  http://cr.openjdk.java.net/~prappo/8151678/webrev.01/
> > > >
> > >
> > > Changes look good!
> > >
> > > best regards,
> > >
> > > -- daniel
> >
>
>

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

> On 6 Sep 2019, at 17:11, Martin Buchholz  wrote:
> 
> Bigger picture: all of this timeout-fiddling infrastructure should be in a 
> test library.  It's not easy!
> 
> test/jdk/java/util/concurrent/tck effectively has its own test library, for 
> historical reasons.

Agreed. It is not an easy task. I'd address this concern of having library 
level support for measuring timeouts and asserting threads blocking in a 
separate bug. For now, I'm afraid I have to reinvent the wheel. 

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

> On 6 Sep 2019, at 15:59, Martin Buchholz  wrote:
> 
> I took another look at LdapTimeoutTest.java.

Many thanks!

> I was surprised to see RIGHT_MARGIN.  In jsr166 we succeed in having one 
> timeout that is long enough to "never happen".  I'm still advocating the 10 
> second value.
> 
> I was surprised to see LEFT_MARGIN.  We just fixed Thread.sleep, so we have 
> no known problems with JDK methods returning early - you can trust timed get!
> You start measuring, by calling nanoTime, before you start the activity you 
> are measuring, so there should be no need for LEFT_MARGIN.

You raised many good points. Let me try to address them.

1. RIGHT_MARGIN is not used for checking that the activity has stuck infinitely 
(assertIncompletion). INFINITY_MILLIS is used for that. RIGHT_MARGIN is used 
for checking that the activity takes some predefined amount of time (roughly).

2. As for the numeric value of INFINITY_MILLIS, the reason I chose 20 seconds 
is twofold. Firstly, the code under test is subject to different timeouts. 
Every timeout value should be distinctive. Otherwise, how would I differentiate 
between them? For example, if I chose INFINITY_MILLIS to be 10 seconds how 
would I know if the code is stuck due to the read timeout of 10 seconds or the 
"infinite timeout"? Secondly, I must take into account slow machines. Really 
slow virtual machines. Hence, minimal timeouts (read/connect) have a magnitude 
of seconds and tens of seconds and the "rightmost", infinite timeout, is 20 
seconds.

3. LEFT_MARGIN might no longer be needed due to the fact that no timed methods 
return early (actually there is a comment about it inside those two assert 
methods).

> We have some fresh thread-awaiting code here:
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?view=markup#l1443

Interesting.

> Instead of communicating startTime from the test thread back to the main 
> thread, I would do my loMillis checking in the test thread, and hiMillis 
> checking in the main thread, like e.g. compare with a fresh test method 
> testTimedOffer
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/ArrayBlockingQueueTest.java?view=markup#l394

Understood. However, that might be a matter of taste. I prefer to have all the 
calculations and error handling in one place. Unless there's a good reason I 
wouldn't change it.

> Timeouts should be adjusted via Utils.adjustTimeout

That makes perfect sense. I never knew this method existed. Thanks!

> On Fri, Sep 6, 2019 at 4:31 AM Pavel Rappo  wrote:
> Martin, thanks for having a look at it.
> 
> I'd appreciate if you could have a look at the timeout measuring mechanics in 
> assertCompletion/assertIncompletion specifically, maybe to spot something 
> that is grossly inadequate.
> 
> I tried to accommodate some usual suspects of timeout measurements failures. 
> I understand that since we're not working with real-time systems, my attempts 
> to build bullet-proof measurement mechanics are futile.
> 
> -Pavel
> 
> > On 30 Aug 2019, at 18:19, Martin Buchholz  wrote:
> > 
> > Not really a review, but:
> > 
> > For many years we've been using 10 seconds (scaled by timeout factor) as a 
> > duration long enough that a timeout is a real failure.
> > Which is close to your own 20 seconds.  Of course, no value is surely safe.
> > 
> > Probably, parallel testing infrastructure for timeouts should be a test 
> > library method.  I do something similar in JSR166TestCase
> > 
> > /**
> >  * Runs all the given actions in parallel, failing if any fail.
> >  * Useful for running multiple variants of tests that are
> >  * necessarily individually slow because they must block.
> >  */
> > void testInParallel(Action ... actions) {
> > ExecutorService pool = Executors.newCachedThreadPool();
> > try (PoolCleaner cleaner = cleaner(pool)) {
> > 
> > On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs  
> > wrote:
> > On 30/08/2019 13:54, Pavel Rappo wrote:
> > > Updated,
> > > 
> > >  http://cr.openjdk.java.net/~prappo/8151678/webrev.01/
> > > 
> > 
> > Changes look good!
> > 
> > best regards,
> > 
> > -- daniel
> 

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

Bigger picture: all of this timeout-fiddling infrastructure should be in a
test library.  It's not easy!

test/jdk/java/util/concurrent/tck effectively has its own test library, for
historical reasons.

On Fri, Sep 6, 2019 at 7:59 AM Martin Buchholz  wrote:

> I took another look at LdapTimeoutTest.java.
>
> I was surprised to see RIGHT_MARGIN.  In jsr166 we succeed in having one
> timeout that is long enough to "never happen".  I'm still advocating the 10
> second value.
>
> I was surprised to see LEFT_MARGIN.  We just fixed Thread.sleep, so we
> have no known problems with JDK methods returning early - you can trust
> timed get!
> You start measuring, by calling nanoTime, before you start the activity
> you are measuring, so there should be no need for LEFT_MARGIN.
>
> We have some fresh thread-awaiting code here:
>
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?view=markup#l1443
>
> Instead of communicating startTime from the test thread back to the main
> thread, I would do my loMillis checking in the test thread, and hiMillis
> checking in the main thread, like e.g. compare with a fresh test
> method testTimedOffer
>
> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/ArrayBlockingQueueTest.java?view=markup#l394
>
> Timeouts should be adjusted via Utils.adjustTimeout
>
>
> On Fri, Sep 6, 2019 at 4:31 AM Pavel Rappo  wrote:
>
>> Martin, thanks for having a look at it.
>>
>> I'd appreciate if you could have a look at the timeout measuring
>> mechanics in assertCompletion/assertIncompletion specifically, maybe to
>> spot something that is grossly inadequate.
>>
>> I tried to accommodate some usual suspects of timeout measurements
>> failures. I understand that since we're not working with real-time systems,
>> my attempts to build bullet-proof measurement mechanics are futile.
>>
>> -Pavel
>>
>> > On 30 Aug 2019, at 18:19, Martin Buchholz  wrote:
>> >
>> > Not really a review, but:
>> >
>> > For many years we've been using 10 seconds (scaled by timeout factor)
>> as a duration long enough that a timeout is a real failure.
>> > Which is close to your own 20 seconds.  Of course, no value is surely
>> safe.
>> >
>> > Probably, parallel testing infrastructure for timeouts should be a test
>> library method.  I do something similar in JSR166TestCase
>> >
>> > /**
>> >  * Runs all the given actions in parallel, failing if any fail.
>> >  * Useful for running multiple variants of tests that are
>> >  * necessarily individually slow because they must block.
>> >  */
>> > void testInParallel(Action ... actions) {
>> > ExecutorService pool = Executors.newCachedThreadPool();
>> > try (PoolCleaner cleaner = cleaner(pool)) {
>> >
>> > On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs 
>> wrote:
>> > On 30/08/2019 13:54, Pavel Rappo wrote:
>> > > Updated,
>> > >
>> > >  http://cr.openjdk.java.net/~prappo/8151678/webrev.01/
>> > >
>> >
>> > Changes look good!
>> >
>> > best regards,
>> >
>> > -- daniel
>>
>>

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

I took another look at LdapTimeoutTest.java.

I was surprised to see RIGHT_MARGIN.  In jsr166 we succeed in having one
timeout that is long enough to "never happen".  I'm still advocating the 10
second value.

I was surprised to see LEFT_MARGIN.  We just fixed Thread.sleep, so we have
no known problems with JDK methods returning early - you can trust timed
get!
You start measuring, by calling nanoTime, before you start the activity you
are measuring, so there should be no need for LEFT_MARGIN.

We have some fresh thread-awaiting code here:
http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?view=markup#l1443

Instead of communicating startTime from the test thread back to the main
thread, I would do my loMillis checking in the test thread, and hiMillis
checking in the main thread, like e.g. compare with a fresh test
method testTimedOffer
http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/ArrayBlockingQueueTest.java?view=markup#l394

Timeouts should be adjusted via Utils.adjustTimeout


On Fri, Sep 6, 2019 at 4:31 AM Pavel Rappo  wrote:

> Martin, thanks for having a look at it.
>
> I'd appreciate if you could have a look at the timeout measuring mechanics
> in assertCompletion/assertIncompletion specifically, maybe to spot
> something that is grossly inadequate.
>
> I tried to accommodate some usual suspects of timeout measurements
> failures. I understand that since we're not working with real-time systems,
> my attempts to build bullet-proof measurement mechanics are futile.
>
> -Pavel
>
> > On 30 Aug 2019, at 18:19, Martin Buchholz  wrote:
> >
> > Not really a review, but:
> >
> > For many years we've been using 10 seconds (scaled by timeout factor) as
> a duration long enough that a timeout is a real failure.
> > Which is close to your own 20 seconds.  Of course, no value is surely
> safe.
> >
> > Probably, parallel testing infrastructure for timeouts should be a test
> library method.  I do something similar in JSR166TestCase
> >
> > /**
> >  * Runs all the given actions in parallel, failing if any fail.
> >  * Useful for running multiple variants of tests that are
> >  * necessarily individually slow because they must block.
> >  */
> > void testInParallel(Action ... actions) {
> > ExecutorService pool = Executors.newCachedThreadPool();
> > try (PoolCleaner cleaner = cleaner(pool)) {
> >
> > On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs 
> wrote:
> > On 30/08/2019 13:54, Pavel Rappo wrote:
> > > Updated,
> > >
> > >  http://cr.openjdk.java.net/~prappo/8151678/webrev.01/
> > >
> >
> > Changes look good!
> >
> > best regards,
> >
> > -- daniel
>
>

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

Martin, thanks for having a look at it.

I'd appreciate if you could have a look at the timeout measuring mechanics in 
assertCompletion/assertIncompletion specifically, maybe to spot something that 
is grossly inadequate.

I tried to accommodate some usual suspects of timeout measurements failures. I 
understand that since we're not working with real-time systems, my attempts to 
build bullet-proof measurement mechanics are futile.

-Pavel

> On 30 Aug 2019, at 18:19, Martin Buchholz  wrote:
> 
> Not really a review, but:
> 
> For many years we've been using 10 seconds (scaled by timeout factor) as a 
> duration long enough that a timeout is a real failure.
> Which is close to your own 20 seconds.  Of course, no value is surely safe.
> 
> Probably, parallel testing infrastructure for timeouts should be a test 
> library method.  I do something similar in JSR166TestCase
> 
> /**
>  * Runs all the given actions in parallel, failing if any fail.
>  * Useful for running multiple variants of tests that are
>  * necessarily individually slow because they must block.
>  */
> void testInParallel(Action ... actions) {
> ExecutorService pool = Executors.newCachedThreadPool();
> try (PoolCleaner cleaner = cleaner(pool)) {
> 
> On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs  wrote:
> On 30/08/2019 13:54, Pavel Rappo wrote:
> > Updated,
> > 
> >  http://cr.openjdk.java.net/~prappo/8151678/webrev.01/
> > 
> 
> Changes look good!
> 
> best regards,
> 
> -- daniel

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Martin Buchholz]

Not really a review, but:

For many years we've been using 10 seconds (scaled by timeout factor) as a
duration long enough that a timeout is a real failure.
Which is close to your own 20 seconds.  Of course, no value is surely safe.

Probably, parallel testing infrastructure for timeouts should be a test
library method.  I do something similar in JSR166TestCase

/**
 * Runs all the given actions in parallel, failing if any fail.
 * Useful for running multiple variants of tests that are
 * necessarily individually slow because they must block.
 */
void testInParallel(Action ... actions) {
ExecutorService pool = Executors.newCachedThreadPool();
try (PoolCleaner cleaner = cleaner(pool)) {

On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs 
wrote:

> On 30/08/2019 13:54, Pavel Rappo wrote:
> > Updated,
> >
> >  http://cr.openjdk.java.net/~prappo/8151678/webrev.01/
> >
>
> Changes look good!
>
> best regards,
>
> -- daniel
>

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Daniel Fuchs]

On 30/08/2019 13:54, Pavel Rappo wrote:

Updated,

 http://cr.openjdk.java.net/~prappo/8151678/webrev.01/



Changes look good!

best regards,

-- daniel

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

Updated,

http://cr.openjdk.java.net/~prappo/8151678/webrev.01/

> On 30 Aug 2019, at 12:28, Daniel Fuchs  wrote:
> 
> Hi Pavel,
> 
> Looks probably OK ;-)
> 
> One thing though is that it's not always guaranteed that
> InetAddress.getByName("localhost") will resolve to the same
> address than that returned by InetAddess.getLoopbackAddress(),
> so it may be better to use the literal address (which may be
> an IPv6 literal requiring enclosing "[" "]") rather than
> the hostname when building the LDAP URL.
> 
> best regards,
> 
> -- daniel
> 
> On 30/08/2019 11:53, Pavel Rappo wrote:
>> //
>> // Dear maintainer:
>> //
>> // Once you are done trying to 'optimize' this 
>> routine,
>> // and have realized what a terrible mistake that 
>> was,
>> // please increment the following counter as a 
>> warning
>> // to the next guy:
>> //
>> // total_hours_wasted_here = 42
>> //
>> --- 
>> [1]
>> Hello,
>> Please review the following change:
>>  http://cr.openjdk.java.net/~prappo/8151678/webrev.00/
>> Testing timeouts is not an easy task. The test in question has always been
>> problematic. There have been many attempts to address the unreliability of 
>> that
>> test and during its lifetime the test has been tagged (and subsequently
>> untagged) as intermittent and eventually was put into the problem list.
>> The proposed change overhauls the test and patches the code that the test
>> exercises.
>> The code change addresses the multithreading issue associated with 8160768 
>> [2].
>> This issue manifests itself as different runtime exceptions that are thrown
>> rather quickly from calls that the test expects to block for a substantial
>> amount of time. Since that test catches this issue reliably, no new tests are
>> introduced.
>> The overhauled test is excluded from the problem list. Before doing so I had 
>> run
>> the test on all the platforms for some 40,000 (4.0E4) times. I have not
>> encountered any failures.
>> And yet I'm under no illusion that this change fixes all the issues. I 
>> strongly
>> suspect that the test will eventually fail again. I’ve attempted to prepare 
>> for
>> that by adding extra diagnostic outputs and improving the usefulness of 
>> traces.
>> -Pavel
>> ---
>> [1] 
>> https://stackoverflow.com/questions/184618/what-is-the-best-comment-in-source-code-you-have-ever-encountered
>> [2] 8160768: Add capability to custom resolve host/domain names within the 
>> default JNDI LDAP provider
> 

Re: RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Daniel Fuchs]

Hi Pavel,

Looks probably OK ;-)

One thing though is that it's not always guaranteed that
InetAddress.getByName("localhost") will resolve to the same
address than that returned by InetAddess.getLoopbackAddress(),
so it may be better to use the literal address (which may be
an IPv6 literal requiring enclosing "[" "]") rather than
the hostname when building the LDAP URL.

best regards,

-- daniel

On 30/08/2019 11:53, Pavel Rappo wrote:

 //
 // Dear maintainer:
 //
 // Once you are done trying to 'optimize' this routine,
 // and have realized what a terrible mistake that was,
 // please increment the following counter as a warning
 // to the next guy:
 //
 // total_hours_wasted_here = 42
 //
 --- [1]

Hello,

Please review the following change:

  http://cr.openjdk.java.net/~prappo/8151678/webrev.00/

Testing timeouts is not an easy task. The test in question has always been
problematic. There have been many attempts to address the unreliability of that
test and during its lifetime the test has been tagged (and subsequently
untagged) as intermittent and eventually was put into the problem list.

The proposed change overhauls the test and patches the code that the test
exercises.

The code change addresses the multithreading issue associated with 8160768 [2].
This issue manifests itself as different runtime exceptions that are thrown
rather quickly from calls that the test expects to block for a substantial
amount of time. Since that test catches this issue reliably, no new tests are
introduced.

The overhauled test is excluded from the problem list. Before doing so I had run
the test on all the platforms for some 40,000 (4.0E4) times. I have not
encountered any failures.

And yet I'm under no illusion that this change fixes all the issues. I strongly
suspect that the test will eventually fail again. I’ve attempted to prepare for
that by adding extra diagnostic outputs and improving the usefulness of traces.

-Pavel

---
[1] 
https://stackoverflow.com/questions/184618/what-is-the-best-comment-in-source-code-you-have-ever-encountered
[2] 8160768: Add capability to custom resolve host/domain names within the 
default JNDI LDAP provider