[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy #14078

lhotari · 2022-01-31T12:59:37Z

Motivation

Pulsar Proxy can get into a state where it stops proxying Broker connections while Admin API proxying keeps working.
The proxy logs are filled with this type of warnings:

[pulsar-proxy-io-2-1] WARN  org.apache.pulsar.client.impl.ConnectionPool - Failed to open connection to pulsar-dev-broker/172.20.4.120:6650 : io.netty.channel.AbstractChannel$AnnotatedConnectException:
connect(.      .) failed: Cannot assign requested address: pulsar-dev-broker.pulsar.svc.cluster.local/172.20.4.120:6650

The "Cannot assign requested address" error message is a sign of a port exhaustion issue where there are many connections open, possibly hanging.

Additional context

One possible reason for the broken hanging connections could be a race condition that shows up in logs this way:

[pulsar-proxy-io-2-3] WARN  io.netty.channel.DefaultChannelPipeline - An exceptionCaught() event was fired, and it reached at the tail of the pipeline. It usually means the last handler in the pipeline did not handle the exception.
java.lang.UnsupportedOperationException: null
        at org.apache.pulsar.common.protocol.PulsarDecoder.handleProducer(PulsarDecoder.java:479)
        at org.apache.pulsar.common.protocol.PulsarDecoder.channelRead(PulsarDecoder.java:193)
        at org.apache.pulsar.proxy.server.ProxyConnection.channelRead(ProxyConnection.java:193)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]

This is reported as #13923 and it is fixed as part of the same PR.

Modifications

Optimize the proxy connection to fail-fast if the target broker isn't active
- This reduces the number of hanging connections when unavailable brokers aren't unnecessarily attempted to be reached.
- Pulsar client will retry connecting after a back off timeout
Fixes the race condition in the Pulsar Proxy when opening a connection since that
could lead to invalid states and hanging connections
Add connect timeout handling to proxy connection
- default to 10000 ms which is also the default of client's connect timeout
Add read timeout handling to incoming connection and proxied connection
- the ping/pong keepalive messages should prevent the timeout happening,
  however the connection might be in a state where keepalives aren't happening.
  - therefore, it's better to have a connection level read timeout to prevent broken connections left
    hanging in the proxy

eolivelli

LGTM
I left some minor suggestions

we should cherry pick this patch to 2.9 and 2.8 branches

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/DirectProxyHandler.java

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java

- enables service discovery in the proxy - required by apache/pulsar#14078 changes

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java

lhotari · 2022-02-04T16:00:21Z

@merlimat There are some test failures, which I'll address next week. Please review the high level approach to see if that's fine.

Fixes apache#14075 Fixes apache#13923 - Optimize the proxy connection to fail-fast if the target broker isn't active - This reduces the number of hanging connections when unavailable brokers aren't unnecessarily attempted to be reached. - Pulsar client will retry connecting after a back off timeout - Fixes the race condition in the Pulsar Proxy when opening a connection since that could lead to invalid states and hanging connections - Add connect timeout handling to proxy connection - default to 10000 ms which is also the default of client's connect timeout - Add read timeout handling to incoming connection and proxied connection - the ping/pong keepalive messages should prevent the timeout happening, however it's possible that the connection is in a state where keepalives aren't happening. - therefore it's better to have a connection level read timeout prevent broken connections left hanging in the proxy

- the test wasn't testing the proxy at all

pulsar-proxy/pom.xml

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java

eolivelli

LGTM

michaeljmarshall

LGTM. Left one comment about a test for ports.

michaeljmarshall · 2022-02-07T18:17:36Z

pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/BrokerProxyValidatorTest.java

+import org.apache.curator.shaded.com.google.common.net.InetAddresses;
+import org.testng.annotations.Test;
+
+public class BrokerProxyValidatorTest {


I think it might be appropriate to add a test named shouldPreventInvalidPort.

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java

…ache#14078) (cherry picked from commit 640b4e6)

…4078) (cherry picked from commit 640b4e6) (cherry picked from commit 3d2e6ce)

…4078) (cherry picked from commit 640b4e6)

…ache#14078) (cherry picked from commit 640b4e6) (cherry picked from commit 3d2e6ce) (cherry picked from commit b3bac91)

…ache#14078) (cherry picked from commit 640b4e6) (cherry picked from commit 3d2e6ce)

…ache#14078)

lhotari added type/bug The PR fixed a bug or issue reported a bug area/proxy doc-not-needed Your PR changes do not impact docs labels Jan 31, 2022

lhotari requested review from merlimat, addisonj, eolivelli, codelipenghui, dave2wave and michaeljmarshall January 31, 2022 12:59

lhotari self-assigned this Jan 31, 2022

eolivelli approved these changes Jan 31, 2022

View reviewed changes

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/DirectProxyHandler.java Show resolved Hide resolved

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java Show resolved Hide resolved

lhotari force-pushed the lh-fix-port-exhaustion-in-proxy branch from c51856a to e4b4081 Compare January 31, 2022 14:39

lhotari added release/2.10.0 release/2.7.5 release/2.8.3 release/2.9.3 labels Jan 31, 2022

lhotari added this to the 2.10.0 milestone Jan 31, 2022

merlimat reviewed Jan 31, 2022

View reviewed changes

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java Show resolved Hide resolved

lhotari force-pushed the lh-fix-port-exhaustion-in-proxy branch from e4b4081 to d8c21cf Compare January 31, 2022 16:25

lhotari added a commit to lhotari/pulsar-helm-chart that referenced this pull request Jan 31, 2022

[Proxy] Configure metadata settings for Proxy

a264093

- enables service discovery in the proxy - required by apache/pulsar#14078 changes

lhotari mentioned this pull request Jan 31, 2022

[Proxy] Configure metadata settings for Proxy apache/pulsar-helm-chart#221

Closed

lhotari commented Jan 31, 2022

View reviewed changes

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java Show resolved Hide resolved

lhotari marked this pull request as draft February 2, 2022 20:48

lhotari force-pushed the lh-fix-port-exhaustion-in-proxy branch from d8c21cf to 01f4d47 Compare February 4, 2022 15:40

lhotari added 5 commits February 7, 2022 10:33

Add BrokerProxyValidator

d044e0e

Fix test

8419558

Improve error logging

b2efb17

Fix issue with allowing all with wildcard

6694628

Fix ProxyServiceStarterTest

8473a15

- the test wasn't testing the proxy at all

lhotari mentioned this pull request Feb 7, 2022

Fix ProxyServiceStarterTest flaky tests #12344

Merged

lhotari added 2 commits February 7, 2022 13:40

Fix 2 more tests by allowing proxying to any target port

eabbdef

Disable non-TLS port for ProxyServiceTlsStarterTest

b715c4c

lhotari marked this pull request as ready for review February 7, 2022 12:15

lhotari requested review from merlimat and eolivelli February 7, 2022 12:22

eolivelli reviewed Feb 7, 2022

View reviewed changes

pulsar-proxy/pom.xml Outdated Show resolved Hide resolved

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java Outdated Show resolved Hide resolved

Address review comments

f3f5581

eolivelli approved these changes Feb 7, 2022

View reviewed changes

michaeljmarshall approved these changes Feb 7, 2022

View reviewed changes

codelipenghui removed the release/2.10.0 label Feb 8, 2022

merlimat reviewed Feb 8, 2022

View reviewed changes

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java Show resolved Hide resolved

merlimat approved these changes Feb 8, 2022

View reviewed changes

eolivelli merged commit 640b4e6 into apache:master Feb 8, 2022

lhotari added a commit to lhotari/pulsar that referenced this pull request Feb 9, 2022

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy (ap…

3d2e6ce

…ache#14078) (cherry picked from commit 640b4e6)

lhotari added a commit that referenced this pull request Feb 9, 2022

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy (#1…

b3bac91

…4078) (cherry picked from commit 640b4e6) (cherry picked from commit 3d2e6ce)

lhotari added cherry-picked/branch-2.7 Archived: 2.7 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life labels Feb 9, 2022

lhotari added a commit that referenced this pull request Feb 9, 2022

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy (#1…

be806ea

…4078) (cherry picked from commit 640b4e6)

lhotari added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Feb 9, 2022

lhotari added a commit to datastax/pulsar that referenced this pull request Feb 9, 2022

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy (ap…

5741f79

…ache#14078) (cherry picked from commit 640b4e6) (cherry picked from commit 3d2e6ce)

gaoran10 added release/2.9.2 and removed release/2.9.3 labels Feb 16, 2022

lhotari mentioned this pull request Mar 16, 2022

[Proxy] Log warning when opening connection to broker fails #14710

Merged

kwenzh mentioned this pull request Mar 24, 2022

[Proxy] Pulsar Handshake was not completed within timeout, Proxy stops working for proxying Broker connections while Admin API proxying keeps working , #14831

Open

sijie mentioned this pull request Mar 24, 2022

ISSUE-14831: [Proxy] Pulsar Handshake was not completed within timeout, Proxy stops working for proxying Broker connections while Admin API proxying keeps working , streamnative/pulsar-archived#3954

Open

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy (ap…

5e435dd

…ache#14078)

lhotari mentioned this pull request Apr 28, 2022

[Proxy] Fix proxy connection leak when inbound connection closes while connecting is in progress #15366

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy #14078

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy #14078

lhotari commented Jan 31, 2022

eolivelli left a comment

lhotari commented Feb 4, 2022

eolivelli left a comment

michaeljmarshall left a comment •

edited

Loading

michaeljmarshall Feb 7, 2022

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy #14078

[Proxy] Fix port exhaustion and connection issues in Pulsar Proxy #14078

Conversation

lhotari commented Jan 31, 2022

Motivation

Additional context

Modifications

eolivelli left a comment

Choose a reason for hiding this comment

lhotari commented Feb 4, 2022

eolivelli left a comment

Choose a reason for hiding this comment

michaeljmarshall left a comment • edited Loading

Choose a reason for hiding this comment

michaeljmarshall Feb 7, 2022

Choose a reason for hiding this comment

michaeljmarshall left a comment •

edited

Loading