Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][websocket] Add ping support #19203

Merged
merged 6 commits into from
Jan 17, 2023
Merged

[improve][websocket] Add ping support #19203

merged 6 commits into from
Jan 17, 2023

Conversation

nodece
Copy link
Member

@nodece nodece commented Jan 12, 2023

Motivation

When the WebSocket connection is idle timeout, the server disconnects the client. We need to keep alive between the client and the server.

There are two options:

  1. Increasing the idle timeout, but be very unfriendly to users who are not sensitive to time
  2. Sending the ping frame to the client

Some client(browser) doesn't support sending the ping frame to the server, which causes idle timeout, so we only add enable ping feature on the server.

Note: The Jetty automatically processes ping/pong frames from the client by default.

Modifications

  • Implement sending the ping frame to the client
  • Add webSocketPingDurationSeconds config to enable the ping feature

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 12, 2023
@nodece nodece self-assigned this Jan 12, 2023
@nodece nodece requested a review from poorbarcode January 13, 2023 09:52
@nodece
Copy link
Member Author

nodece commented Jan 13, 2023

Ping @poorbarcode

@Technoboy- Technoboy- added this to the 2.12.0 milestone Jan 14, 2023
Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Merging #19203 (b6c6471) into master (246c270) will decrease coverage by 0.45%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19203      +/-   ##
============================================
- Coverage     43.01%   42.56%   -0.46%     
- Complexity    10167    10304     +137     
============================================
  Files           747      773      +26     
  Lines         72255    74509    +2254     
  Branches       7786     8031     +245     
============================================
+ Hits          31080    31714     +634     
- Misses        37577    39077    +1500     
- Partials       3598     3718     +120     
Flag Coverage Δ
unittests 42.56% <ø> (-0.46%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...g/apache/pulsar/broker/service/StreamingStats.java 0.00% <0.00%> (-83.79%) ⬇️
...sar/broker/stats/metrics/ManagedLedgerMetrics.java 23.88% <0.00%> (-76.12%) ⬇️
...rvice/schema/KeyValueSchemaCompatibilityCheck.java 21.62% <0.00%> (-45.95%) ⬇️
...ker/service/schema/exceptions/SchemaException.java 60.00% <0.00%> (-40.00%) ⬇️
...lsar/broker/service/InMemoryRedeliveryTracker.java 58.82% <0.00%> (-35.30%) ⬇️
...ersistentStickyKeyDispatcherMultipleConsumers.java 18.13% <0.00%> (-34.32%) ⬇️
...ervice/persistent/MessageRedeliveryController.java 38.33% <0.00%> (-31.67%) ⬇️
...ulsar/utils/ConcurrentBitmapSortedLongPairSet.java 38.88% <0.00%> (-20.38%) ⬇️
.../org/apache/pulsar/broker/lookup/LookupResult.java 56.66% <0.00%> (-20.00%) ⬇️
...ulsar/broker/namespace/NamespaceEphemeralData.java 68.18% <0.00%> (-18.19%) ⬇️
... and 88 more

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
Copy link
Member Author

nodece commented Jan 16, 2023

/pulsarbot rerun-failure-checks

@nodece
Copy link
Member Author

nodece commented Jan 17, 2023

Remove release/2.9.5, using #19245 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants