Skip to content

fix: s/sliding-stream w/ certain executor configs #241

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rschmukler
Copy link

Fixes an issue where, depending on which thread the source stream was running, the sliding stream would never emit later items downstream.

We now move consumption onto d/loop rather than via the connet-via.

Updates tests to a more imperative variant to try and mimic items coming in faster than consumption.

Fixes an issue where, depending on where the source stream was running,
the sliding stream would never emit later items downstream.

We now move consumption onto `d/loop` rather than via the `connet-via`.

Updates tests to a more imperative variant to try and mimic items coming
in faster than consumption.
@rschmukler rschmukler requested a review from KingMob as a code owner April 23, 2025 20:15
@KingMob KingMob requested a review from DerGuteMoritz April 24, 2025 06:05
@rschmukler
Copy link
Author

rschmukler commented Apr 24, 2025

This PR is probably not worth merging. I am still seeing the behavior it was intended to fix. If you like the code it might be more "idiomatic" per other internal stream implementations, but the underlying issue remains. Will submit a PR once I have a real fix. Sorry for the noise.

@KingMob
Copy link
Collaborator

KingMob commented Apr 24, 2025

@rschmukler No worries, and thanks for the effort!

It might help to open an issue to talk it through first. Either that or discuss it in #aleph/#manifold.

Also, ping @DerGuteMoritz when it's ready, I'm not super-involved in Aleph/Manifold maintenance these days. Probably need to find a more active maintainer...

@rschmukler
Copy link
Author

rschmukler commented Apr 24, 2025

The issue that I am running into is that under heavy production load, the stream that is producing items can end up cycling through the buffer (via take!) faster than rightful other consumers can take a message.

Eg. with a buffer size 1 and a very fast producer, it ends up basically looping over and over trying to drain the buffer and the downstream consumer never actually gets a message.

This might have something to do with (a lack of) fairness guarantees around take! but I'm not too sure.

In its current form I lost two days debugging what was going on; it might be worth deprecating it (and I say that as the original author haha)

@arnaudgeiser
Copy link
Collaborator

In its current form I lost two days debugging what was going on; it might be worth deprecating it (and I say that as the original author haha)

For the record: those functions were introduced quite recently and you might not be the only one encountering this issue. I would be interested to have more feedback from the field.

@KingMob KingMob requested review from arnaudgeiser and removed request for KingMob April 26, 2025 09:42
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