Skip to content

Optimize performance when many new connections arrive #2837

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

Closed
wants to merge 3 commits into from

Conversation

xiexin36
Copy link
Contributor

When many new connections arrive at once, we should accept as soon as possible. accept connections in a for loop until no new connection is ready

@xiexin36 xiexin36 changed the base branch from master to 5.1.x March 22, 2019 07:12
@garyrussell
Copy link
Contributor

Thanks for the contribution, but why not just increase the backlog?

	/**
	 * The number of sockets in the connection backlog. Default 5;
	 * increase if you expect high connection rates.
	 * @param backlog The backlog to set.
	 */
	public void setBacklog(int backlog) {

Also, we don't accept code changes without test cases that exercise the new code.

@xiexin36
Copy link
Contributor Author

xiexin36 commented Mar 22, 2019

@garyrussell Yes, I also tried to set the backlog, even to 20, and I also set the nioHarvestInterval bigger than default, but it did not work. Everytime AbstractConnectionFactory.processNioSelections execute, there are too many things need to do , and the doAccept is the last one.

@xiexin36
Copy link
Contributor Author

xiexin36 commented Mar 28, 2019

  • The number of sockets in the connection backlog. Default 5;
  • increase if you expect high connection rates.
  • @param backlog The backlog to set.
    */
    public void setBacklog(int backlog) {

Also, we don't accept code changes without test cases that exercise the new code.

@garyrussell With the current test ConnectionFactoryTests.testObtainConnectionIdsNio, the new code can be converaged, I have run the test.

@xiexin36 xiexin36 closed this Mar 28, 2019
@xiexin36 xiexin36 reopened this Mar 28, 2019
@garyrussell
Copy link
Contributor

OK; thanks for the contribution.

@garyrussell garyrussell self-assigned this Mar 28, 2019
@artembilan artembilan added this to the 5.1.4 milestone Mar 28, 2019
garyrussell added a commit to garyrussell/spring-integration that referenced this pull request Mar 28, 2019
See spring-projects#2837

Give priority to accepting new connections over reading data from
existing connections.
Add a flag to allow users to revert to the previous behavior of
giving reads priority.

**cherry-pick to 5.1.x**
@garyrussell
Copy link
Contributor

@xiexin36 Thanks for the contribution, but I have some concerns about read starvation, so I made this optional (but true by default).

Replaced by #2865

garyrussell added a commit to garyrussell/spring-integration that referenced this pull request Mar 28, 2019
See spring-projects#2837

Give priority to accepting new connections over reading data from
existing connections.
Add a flag to allow users to revert to the previous behavior of
giving reads priority.

**cherry-pick to 5.1.x**
garyrussell added a commit to garyrussell/spring-integration that referenced this pull request Mar 28, 2019
See spring-projects#2837

Give priority to accepting new connections over reading data from
existing connections.
Add a flag to allow users to revert to the previous behavior of
giving reads priority.

**cherry-pick to 5.1.x**
artembilan pushed a commit that referenced this pull request Mar 28, 2019
See #2837

Give priority to accepting new connections over reading data from
existing connections.
Add a flag to allow users to revert to the previous behavior of
giving reads priority.

**cherry-pick to 5.1.x**
artembilan pushed a commit that referenced this pull request Mar 28, 2019
See #2837

Give priority to accepting new connections over reading data from
existing connections.
Add a flag to allow users to revert to the previous behavior of
giving reads priority.

**cherry-pick to 5.1.x**

# Conflicts:
#	spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioServerConnectionFactory.java
#	src/reference/asciidoc/whats-new.adoc
#   TcpNioConnectionTests.java
@xiexin36
Copy link
Contributor Author

@xiexin36 Thanks for the contribution, but I have some concerns about read starvation, so I made this optional (but true by default).

Replaced by #2865

@garyrussell Great job ! And when the 5.1.4 version will be release?

@garyrussell
Copy link
Contributor

Currently scheduled for next Tuesday (April 2).

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

Successfully merging this pull request may close these issues.

3 participants