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

Ensure BufferingChannelTransport flushing task does not block event loop #2399

Conversation

hmhagberg
Copy link
Contributor

If data is written to BufferingChannelTransport queue faster than it can be flushed, the flushing task can take arbitrarily long to complete. This prevents any other queued task from running.

This patch ensures that a new task is created when MaxFlushSize (currently 128) messages are flushed so other queued tasks are executed sooner.

Fixes #2398

Signed-off-by: Henri Hagberg [email protected]

@adleong
Copy link
Member

adleong commented Jun 4, 2020

Thanks @hmhagberg! My main concern here is that I wonder if this makes it possible for items to become stuck in the writeQueue and not get flushed out until the next write occurs.

@hmhagberg
Copy link
Contributor Author

I don't think that is an issue. At the end of flush there is

    if (!writeQueue.isEmpty) {
      scheduleFlush()
    }

so a new task is immediately scheduled if there are unwritten items.

The main difference here is that if anything else was added to event loop queue (like ping task) that will be executed first before flushing is continued.

@cpretzer
Copy link
Contributor

Closing this in favor of the workaround specified in #2398

Set -com.twitter.finagle.liveness.sessionFailureDetector=none

@cpretzer cpretzer closed this Jun 11, 2020
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.

Failure detector closes connection if linkerd receives data faster than it can write
3 participants