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

[nodece]

Fix #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

[lhotari]

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.

[lhotari]

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.

Sorry for the noise. Yes, this PR helps. #15382 was a similar change to Ping/Pong.

[lhotari]

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 kubernetes/kubernetes#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 #20026. The previous improvement was #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 (kubernetes/kubernetes#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 (kubernetes/kubernetes#101607). There's also kubernetes/kubernetes#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 connections can just stall GitHub instead of receiving a RST (kubernetes/kubernetes#124290). And kubernetes/kubernetes#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.

Many of the issues are simply not easy to resolve as explained in the discussion that Michael had on the issue (for example comment 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.

[lhotari]

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.

Claude's comments:

eBPF-based kube-proxy replacements like Cilium (and Calico in eBPF mode) are available on all major managed Kubernetes services — EKS, GKE, and AKS. However, the level of integration and support varies:

• GKE offers "GKE Dataplane V2" which is Cilium under the hood, provided as a first-party Google option. When you select Dataplane V2 at cluster creation, you get Cilium with kube-proxy replacement managed by Google.

• AKS offers "Azure CNI Powered by Cilium" as a first-party managed option. When you create a cluster with --network-dataplane cilium, AKS deploys Cilium as the data plane and does not use kube-proxy at all. AKS manages the Cilium configuration — customers who need more control can use AKS BYOCNI and install Cilium manually instead. AKS also supports a BYOCNI mode for fully self-managed Cilium installations.

• EKS supports Cilium either as a full CNI replacement (replacing the default AWS VPC CNI) or in "chaining" mode alongside the VPC CNI. Replacing kube-proxy with Cilium on EKS is well-documented and supported, but it's a bring-your-own setup — AWS doesn't manage Cilium for you.

The default on EKS remains kube-proxy with the AWS VPC CNI. On GKE and AKS, Cilium-based dataplanes are available as first-party options at cluster creation time, though kube-proxy-based configurations are still the default unless explicitly selected otherwise.

It's worth noting that "recommendation" may be too strong — it's more accurate to say that eBPF-based kube-proxy replacement is increasingly recognized as a best practice for clusters at scale or clusters that need better connection lifecycle management. For smaller clusters, kube-proxy works fine and is simpler to operate.

Cilium isn't the only option either — Calico's eBPF mode also provides connect-time load balancing and can replace kube-proxy. The broader trend is toward eBPF-based alternatives for service networking, with Cilium being the most prominent.

[nodece]

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