Flaky-test: ConnectionPoolTest.testSetProxyToTargetBrokerAddress

[michaeljmarshall]

Search before asking

• I searched in the issues and found nothing similar.

Example failure

https://github.com/apache/pulsar/actions/runs/4933518332/jobs/8817664984?pr=20289#step:10:747

Exception stacktrace

   Error:  org.apache.pulsar.client.impl.ConnectionPoolTest.testSetProxyToTargetBrokerAddress  Time elapsed: 0.085 s  <<< FAILURE!
  java.lang.AssertionError: expected [broker] but found [proxy]
  	at org.testng.Assert.fail(Assert.java:110)
  	at org.testng.Assert.failNotEquals(Assert.java:1577)
  	at org.testng.Assert.assertEqualsImpl(Assert.java:149)
  	at org.testng.Assert.assertEquals(Assert.java:131)
  	at org.testng.Assert.assertEquals(Assert.java:655)
  	at org.testng.Assert.assertEquals(Assert.java:665)
  	at org.apache.pulsar.client.impl.ConnectionPoolTest.testSetProxyToTargetBrokerAddress(ConnectionPoolTest.java:219)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
  	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:139)
  	at org.testng.internal.invokers.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:47)
  	at org.testng.internal.invokers.InvokeMethodRunnable.call(InvokeMethodRunnable.java:76)
  	at org.testng.internal.invokers.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
  	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
  	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
  	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
  	at java.base/java.lang.Thread.run(Thread.java:833)

Are you willing to submit a PR?

• I'm willing to submit a PR!

[michaeljmarshall]

I saw this with #20289, but there doesn't appear to be any correlation. I cannot get it to fail locally. @nicoloboschi - this is from a test you recently added. Have you noticed any flakiness?

[lhotari]

the test has a problem:

pulsar/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/ConnectionPoolTest.java

Lines 186 to 192 in e286339

	if (isProxy) {
	result.add(new InetSocketAddress("localhost", brokerPort));
	result.add(InetSocketAddress.createUnresolved("proxy", brokerPort));
	} else {
	result.add(new InetSocketAddress("127.0.0.1", brokerPort));
	result.add(InetSocketAddress.createUnresolved("broker", brokerPort));
	}

new InetSocketAddress("localhost", brokerPort) and new InetSocketAddress("127.0.0.1", brokerPort) will collide and that's why test fails in certain case.

I guess that one way to fix this would be to use different loopback address.
IPv4 network standards reserve the entire address block 127.0.0.0/8 (more than 16 million addresses) for loopback purposes.
For example 127.0.0.101 and 127.0.0.102 could be used.

[lhotari]

This issue reproduces locally with @Test(invocationCount = 50, threadPoolSize = 10) for me.

[lhotari]

The change to use 127.0.0.101 and 127.0.0.102 doesn't fix the issue.

This looks like a bug in the connection pool logic itself. The key in the pool should use both the logicalAddress and physicalAddress? I wonder how this could have worked in the first place?

[lhotari]

with conf.setConnectionsPerBroker(1); the test fails more often.

[lhotari]

the flakiness goes away when I make ClientCnx.close synchronous:

    public void close() {
       if (ctx != null) {
           try {
               ctx.close().sync();
           } catch (InterruptedException e) {
               Thread.currentThread().interrupt();
           }
       }
    }

It seems that it doesn't make sense to close the connection since that removes the connection from the connection pool. The connection pool in Pulsar client isn't the connection pool you would expect it to be. It's not like a JDBC datasource connection pool.

[lhotari]

Changes to the test that make it fail consistently: lhotari@1018fb9

This seems to reproduce the bug in the connection pool. The connection pool should take the proxy target into account.

[lhotari]

After all, I came to the conclusion that the test was wrong. Here's the PR which contains the description and fix: #20293