You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "nodece (via GitHub)" <gi...@apache.org> on 2026/04/14 10:13:41 UTC

[PR] [improve][broker] Close connection when close consumer write fails [pulsar]

nodece opened a new pull request, #25520:
URL: https://github.com/apache/pulsar/pull/25520

   Main Issue: #25146
   
   ### Motivation
   
   When the broker asks a client to close a consumer, if the `CloseConsumer` command does not reach the client, the client may keep the consumer alive locally while the broker has already removed it, leading to a silent inconsistent state.
   
   ### Modifications
   
   - update broker-side consumer close handling to close the whole connection when sending `CloseConsumer` fails
   - keep the existing protocol behavior for older clients that already fall back to closing the connection
   - add a broker test covering the failed `CloseConsumer` write path
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   This change added tests and can be verified as follows:
   
   - Added a `ServerCnxTest` case for failed `CloseConsumer` writes to ensure the connection is closed
   
   ### Does this pull request potentially affect one of the following parts:
   
   - [ ] Dependencies (add or upgrade a dependency)
   - [ ] The public API
   - [ ] The schema
   - [ ] The default values of configurations
   - [ ] The threading model
   - [ ] The binary protocol
   - [ ] The REST endpoints
   - [ ] The admin CLI options
   - [ ] The metrics
   - [ ] Anything that affects deployment
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve][broker] Close connection when close consumer write fails [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #25520:
URL: https://github.com/apache/pulsar/pull/25520#issuecomment-4247605683

   Some additional details from Claude:
   > Cilium's Socket-LB, when used as a kube-proxy replacement, mitigates many of the stale connection and conntrack problems that affect kube-proxy in both iptables and IPVS modes. Because Socket-LB performs service-to-backend translation at the socket layer (at connect() time for TCP) rather than per-packet NAT, it avoids creating conntrack entries for service translation entirely. This eliminates the class of problems where stale conntrack entries cause connections to be black-holed or forwarded to dead backends after node reboots, pod restarts, or endpoint changes.
   Additionally, because Cilium tracks which sockets are connected to which backends in BPF maps, it can proactively destroy sockets connected to removed backends — using either the kernel's SOCK_DESTROY API (via netlink, requiring CONFIG_INET_DIAG_DESTROY) or the newer bpf_sock_destroy() kfunc (kernel 6.5+, being adopted in Cilium 1.17+). This is something neither kube-proxy iptables mode nor IPVS mode can do, since they operate at the packet level and have no visibility into application sockets.
   That said, Socket-LB doesn't cover all traffic paths — north-south traffic (external to cluster) and some scenarios like hostNamespaceOnly mode still use eBPF-based per-packet NAT at the tc/XDP layer, which is closer to what kube-proxy does (though still using BPF maps rather than iptables/IPVS). The conntrack-related issues are most effectively mitigated for east-west pod-to-service traffic where Socket-LB is fully in play.
   
   It seems that there's a recommendation to replace kube-proxy with Cilium. This option seems to be available also on all major cloud provider managed k8s services (EKS, GKE and AKS). The default option is kube-proxy in most cases.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve][broker] Close connection when close consumer write fails [pulsar]

Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece merged PR #25520:
URL: https://github.com/apache/pulsar/pull/25520


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve][broker] Close connection when close consumer write fails [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #25520:
URL: https://github.com/apache/pulsar/pull/25520#issuecomment-4247208224

   > When the broker asks a client to close a consumer, if the `CloseConsumer` command does not reach the client, the client may keep the consumer alive locally while the broker has already removed it, leading to a silent inconsistent state.
   
   The change looks ok, but I have doubts whether there could be a scenario where the command doesn't reach the client and there's no other error / exception in the logs. Any unhandled exception would result in the connection being closed by [this code in exceptionCaught](https://github.com/apache/pulsar/blob/daa5f8db6ce4befeda0b71b6e1d4ae73f4d56c11/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L535-L553).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve][broker] Close connection when close consumer write fails [pulsar]

Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on PR #25520:
URL: https://github.com/apache/pulsar/pull/25520#issuecomment-4248751021

   @lhotari Thanks for your share, this is useful for me!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve][broker] Close connection when close consumer write fails [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #25520:
URL: https://github.com/apache/pulsar/pull/25520#issuecomment-4247236618

   > > When the broker asks a client to close a consumer, if the `CloseConsumer` command does not reach the client, the client may keep the consumer alive locally while the broker has already removed it, leading to a silent inconsistent state.
   > 
   > The change looks ok, but I have doubts whether there could be a scenario where the command doesn't reach the client and there's no other error / exception in the logs. Any unhandled exception would result in the connection being closed by [this code in exceptionCaught](https://github.com/apache/pulsar/blob/daa5f8db6ce4befeda0b71b6e1d4ae73f4d56c11/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java#L535-L553).
   
   Well, after it, this might help. https://github.com/apache/pulsar/pull/15382 was a similar change to Ping/Pong.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve][broker] Close connection when close consumer write fails [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #25520:
URL: https://github.com/apache/pulsar/pull/25520#issuecomment-4247470364

   >  the client may keep the consumer alive locally while the broker has already removed it
   
   Btw. there are also other cases where the consumer might be alive on the client side (until the next Ping command) which are caused by different reason than what is handled by this PR. They happen especially in Kubernetes environments. One of the unresolved issues in Kubernetes is https://github.com/kubernetes/kubernetes/issues/104098 (this was an issue that Michael was digging into a few years ago) which is challenging. IIRC, the main scenario was rebooting a k8s node. That resulted in problems in many Pulsar clients and stale connections. The problem at that time with k8s node reboots was mitigated with https://github.com/apache/pulsar/pull/20026. The previous improvement was  https://github.com/apache/pulsar/pull/15382, but that has the latency of the keep alive interval.
   
   A long list of related issues in Kubernetes (list compiled with Claude):
   > When a Kubernetes node is rebooted or a pod is rescheduled, TCP connections can become stale because kube-proxy doesn't always clean up conntrack entries for TCP connections. When a node becomes NotReady, kube-proxy can delete conntrack entries for UDP, but doesn't delete conntrack entries for TCP [GitHub](https://github.com/kubernetes/kubernetes/issues/104098) ([kubernetes/kubernetes#104098](https://github.com/kubernetes/kubernetes/issues/104098)). After a node reboot, stale conntrack entries remain, and the external traffic can arrive at the node before kube-proxy has set up the NAT entries [GitHub](https://github.com/kubernetes/kubernetes/issues/118814) ([kubernetes/kubernetes#101607](https://github.com/kubernetes/kubernetes/issues/101607)). There's also [kubernetes/kubernetes#118814](https://github.com/kubernetes/kubernetes/issues/118814) which covers the same issue with IPVS mode. Similarly, when a pod backing a service is restarted and gets a new IP, all existing TCP conne
 ctions can just stall [GitHub](https://github.com/kubernetes/kubernetes/issues/124290) instead of receiving a RST ([kubernetes/kubernetes#124290](https://github.com/kubernetes/kubernetes/issues/124290)). And [kubernetes/kubernetes#100698](https://github.com/kubernetes/kubernetes/issues/100698) documents how established TCP sessions to a leader pod don't get disconnected when the service endpoints change — the conntrack rules for those sessions are not cleared, which is a serious problem for leader/follower setups with long-lived connections [GitHub](https://github.com/kubernetes/kubernetes/issues/100698).
   
   Many of the issues are simply not easy to resolve as explained in the discussion that Michael had on the issue (for example [comment](https://github.com/kubernetes/kubernetes/issues/104098#issuecomment-1518521697) and the following one are at the gist of the problem). There's also a reference https://blog.cloudflare.com/when-tcp-sockets-refuse-to-die/ in the issue comments.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org