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

Fairness across multiple streams #41

Closed
Lupus opened this issue Feb 15, 2020 · 3 comments
Closed

Fairness across multiple streams #41

Lupus opened this issue Feb 15, 2020 · 3 comments

Comments

@Lupus
Copy link
Contributor

Lupus commented Feb 15, 2020

We ran into an issue with fairness when running multiple echo streams on one httpaf-based service. This problem boils down to one stream hogging all CPU power as reads never block on high data rates and event loop does not have a chance to run and schedule others. See ocsigen/lwt#622 for more context.

In our custom Lwt adapter we workaround this by adding Lwt_main.yield () after each read loop iteration along with increase of read buffer size to one megabyte, this way we yield once per each megabyte of data read, which gives good fairness and does not kill performance by endless epolls.

May be similar logic could be baked into Lwt adapter behind some configuration parameters?

@Lupus
Copy link
Contributor Author

Lupus commented May 13, 2020

Actually after back-pressure feature has landed, adding Lwt.pause() inside on_read callback right before scheduling another read callback works as well as our forked Lwt adapter with vanilla httpaf, so we would probably settle on this approach, as it allows us to avoid patching&rebasing gluten upon each release.

@anmonteiro
Copy link
Owner

@Lupus I'm tempted to close this issue as a "won't fix", and instead recommend folks to use Lwt.pause inside their on_read handlers every N reads, like you do in your applications. What do you think?

@Lupus
Copy link
Contributor Author

Lupus commented Oct 9, 2021

Yeah, sounds good to me!

@Lupus Lupus closed this as completed Oct 9, 2021
anmonteiro added a commit to anmonteiro/piaf that referenced this issue Oct 9, 2021
this diff gives running threads a chance to make progress (via
`Lwt.pause`) whenever Piaf is reading a message body from the peer.

related: anmonteiro/httpun#41
anmonteiro added a commit to anmonteiro/piaf that referenced this issue Oct 22, 2021
* Body: yield to other threads when reading

this diff gives running threads a chance to make progress (via
`Lwt.pause`) whenever Piaf is reading a message body from the peer.

related: anmonteiro/httpun#41

* tweaks

* update
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

No branches or pull requests

2 participants