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

[#4205] Correctly set EPOLLOUT flag whe writeBytes(...) was not able … #4216

Closed
wants to merge 1 commit into
base: 4.0
from

Conversation

Projects
None yet
4 participants
@normanmaurer
Member

normanmaurer commented Sep 14, 2015

…to write everything

Motivation:

writeBytes(...) missed to set EPOLLOUT flag when not all bytes were written. This could lead to have the EpollEventLoop not try to flush the remaining bytes once the socket becomes writable again.

Modifications:

  • Move setting EPOLLOUT flag logic to one point so we are sure we always do it.
  • Move OP_WRITE flag logic to one point as well.

Result:

Correctly try to write pending data if socket becomes writable again.

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Sep 14, 2015

Member

@trustin please check

Member

normanmaurer commented Sep 14, 2015

@trustin please check

[#4205] Correctly set EPOLLOUT flag whe writeBytes(...) was not able …
…to write everything

Motivation:

writeBytes(...) missed to set EPOLLOUT flag when not all bytes were written. This could lead to have the EpollEventLoop not try to flush the remaining bytes once the socket becomes writable again.

Modifications:

- Move setting EPOLLOUT flag logic to one point so we are sure we always do it.
- Move OP_WRITE flag logic to one point as well.

Result:

Correctly try to write pending data if socket becomes writable again.
@netkins

This comment has been minimized.

Show comment
Hide comment
@netkins

netkins Sep 15, 2015

TeamCity pull requests :: netty Build 205 is now running

netkins commented on 2290eb2 Sep 15, 2015

TeamCity pull requests :: netty Build 205 is now running

This comment has been minimized.

Show comment
Hide comment
@netkins

netkins Sep 15, 2015

TeamCity pull requests :: netty Build 205 outcome was SUCCESS
Summary: Tests passed: 4765, ignored: 21 Build time: 00:30:25

netkins replied Sep 15, 2015

TeamCity pull requests :: netty Build 205 outcome was SUCCESS
Summary: Tests passed: 4765, ignored: 21 Build time: 00:30:25

@ninja-

This comment has been minimized.

Show comment
Hide comment
@ninja-

ninja- Sep 15, 2015

@normanmaurer ok so the $Entry problem can be separated into two parts. I am sure that the first part is fixed with a quick one-minute test, with this patch. I will test the rest on production and I am really expecting a drop in $Entry count from 5 000 000

ninja- commented Sep 15, 2015

@normanmaurer ok so the $Entry problem can be separated into two parts. I am sure that the first part is fixed with a quick one-minute test, with this patch. I will test the rest on production and I am really expecting a drop in $Entry count from 5 000 000

@ninja-

This comment has been minimized.

Show comment
Hide comment
@ninja-

ninja- Sep 15, 2015

@normanmaurer you can pull(please bump 4.1 beta). If - assuming the application doesn't batch packets and every time calls writeAndFlush it would keep growing to millions, that would need a separate investigation anyway.

ninja- commented Sep 15, 2015

@normanmaurer you can pull(please bump 4.1 beta). If - assuming the application doesn't batch packets and every time calls writeAndFlush it would keep growing to millions, that would need a separate investigation anyway.

@Scottmitch

This comment has been minimized.

Show comment
Hide comment
@Scottmitch

Scottmitch Sep 15, 2015

Member

@ninja- - This does not affect to account for back pressure if that is your question. Just ensure that we write what we have queued when the channel becomes writable.

Member

Scottmitch commented Sep 15, 2015

@ninja- - This does not affect to account for back pressure if that is your question. Just ensure that we write what we have queued when the channel becomes writable.

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Sep 16, 2015

Member

Cherry-picked into 4.0 (7961138), 4.1 (076d4ed) and master (943db1c)

Member

normanmaurer commented Sep 16, 2015

Cherry-picked into 4.0 (7961138), 4.1 (076d4ed) and master (943db1c)

@normanmaurer normanmaurer deleted the epollout_fix branch Sep 16, 2015

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