Skip to content

[Broker/Client] Close connection if a ping or pong message cannot be sent - #15382

Merged
lhotari merged 2 commits into
apache:masterfrom
lhotari:lh-close-connection-when-sending-ping-fails
Apr 29, 2022
Merged

[Broker/Client] Close connection if a ping or pong message cannot be sent#15382
lhotari merged 2 commits into
apache:masterfrom
lhotari:lh-close-connection-when-sending-ping-fails

Conversation

@lhotari

@lhotari lhotari commented Apr 29, 2022

Copy link
Copy Markdown
Member

Motivation

The keep alive implementation used in the Broker and the Java client ignores any failures that happen while writing the Ping message.
In Netty 4+, it's necessary to add a listener to detect failures when writing to outbound channels.
The connection should be closed if a ping message cannot be sent. The future is notified once the OS level TCP/IP stack accepts the message for delivery.

This change will help detect the situation where writing to the outbound channel returns EBADF errno from the OS level and the connection isn't yet completely closed.
EBADF errno maps to java.nio.channels.ClosedChannelException which has this kind of explanation:

Checked exception thrown when an attempt is made to invoke or complete an I/O operation upon channel that is closed, or at least closed to that operation. That this exception is thrown does not necessarily imply that the channel is completely closed. A socket channel whose write half has been shut down, for example, may still be open for reading.

Modifications

  • Add a listener that closes the connection if sending the Ping message fails.
  • Add a listener that closes the connection if sending the Pong message fails.

- the connection should be closed if a ping message cannot be sent

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@lhotari

lhotari commented Apr 29, 2022

Copy link
Copy Markdown
Member Author

I added similar handling for the Pong message.

@lhotari lhotari changed the title [Broker/Client] Close connection if a ping message cannot be sent [Broker/Client] Close connection if a ping or pong message cannot be sent Apr 29, 2022
@lhotari
lhotari merged commit 2ddef95 into apache:master Apr 29, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 29, 2022
…sent (apache#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
lhotari added a commit that referenced this pull request Apr 29, 2022
…sent (#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
lhotari added a commit that referenced this pull request Apr 29, 2022
…sent (#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
@lhotari lhotari added cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life labels Apr 29, 2022
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 29, 2022
…sent (apache#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
lhotari added a commit that referenced this pull request Apr 29, 2022
…sent (#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
lhotari added a commit to datastax/pulsar that referenced this pull request Apr 29, 2022
…sent (apache#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 9, 2022
…sent (apache#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
(cherry picked from commit 5da1637)
Jason918 pushed a commit that referenced this pull request Jul 29, 2022
…sent (#15382)

* [Broker/Client] Close connection if a ping message cannot be sent

- the connection should be closed if a ping message cannot be sent

* Handle write errors for pong messages

(cherry picked from commit 2ddef95)
@Jason918 Jason918 added cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.5 and removed release/2.7.6 labels Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants