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

Allow user to specify max number of pending connections to a server #1001

Merged
merged 2 commits into from Apr 23, 2020

Conversation

PhilipRoman
Copy link
Collaborator

@PhilipRoman PhilipRoman commented Apr 22, 2020

Theoretically I should add this to 1.5.0 milestone, but since this is a non-breaking change and we also have a non-breaking #1000, so maybe there should be a small 1.4.2 release so that people can use these improvements without breaking changes of 1.5.0? I will add the appropriate @​since tags to JavaDoc when this is decided.

Description

Adds a field and two new methods to WebSocketServer: setMaxPendingConnections(int) and getMaxPendingConnections() and uses the value as a parameter when binding the server socket.

Related Issue

Fixes #991

Motivation and Context

This change allows users to configure the maximum number of pending connections.

How Has This Been Tested?

Existing tests run fine.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Adds a field and two new methods to WebSocketServer: setMaxPendingConnections(int) and getMaxPendingConnections() and uses the value as a parameter when binding the server socket.
@marci4
Copy link
Collaborator

marci4 commented Apr 22, 2020

I think there is no need to patch this to 1.4.2, 1.5.0 should be fine

#1000 requires 1.7 API level https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLParameters.html#setEndpointIdentificationAlgorithm-java.lang.String-

Best regards,
Marcel

@marci4 marci4 added this to the Release 1.5.0 milestone Apr 22, 2020
@marci4
Copy link
Collaborator

marci4 commented Apr 22, 2020

I think there is no good way to test this right?

@PhilipRoman
Copy link
Collaborator Author

Well, the functionality is implemented in JDK, so it's their responsibility. The getter/setter logic in our code is trivial, so I think that there is no point in testing it.

Theoretically we could prevent the server socket from accepting incoming sockets and see how many connections it takes to start dropping them, but then we would be simply testing JDK code.

@PhilipRoman
Copy link
Collaborator Author

If the only "breaking change" in 1.5.0 is a newer JDK version, then I agree about putting this in 1.5.0.

@PhilipRoman
Copy link
Collaborator Author

For some reason the test action was running for over 2.5 hours for the last commit, so I stopped it.

@marci4 marci4 merged commit 9d890db into master Apr 23, 2020
@marci4 marci4 deleted the issue-991 branch April 23, 2020 09:30
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.

Add a new WebSocketServer constructor allowing to specify the maximum number of pending connections
2 participants