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

Cannot create a replay sink with zero size #2772

Closed
osi opened this issue Sep 15, 2021 · 5 comments · Fixed by #2787
Closed

Cannot create a replay sink with zero size #2772

osi opened this issue Sep 15, 2021 · 5 comments · Fixed by #2787
Assignees
Labels
status/has-workaround This has a known workaround described type/documentation A documentation update
Milestone

Comments

@osi
Copy link
Contributor

osi commented Sep 15, 2021

Expected Behavior

Can create a sink using

Sinks.many().replay().limit(0)

To get a sink that:

  • Does not store any values for late subscribers
  • Allows error-free publication when no subscribers
  • Allows subscribers to come-and-go

Actual Behavior

Fails with

java.lang.IllegalArgumentException: historySize must be > 0

This is caused by #2741 , specifically,

This is not entirely ideal, but we're making the bet here that nobody actually uses historySize == 0 in the Sinks side of things, since it is a younger API.

I'm using it 😀

@reactorbot reactorbot added the ❓need-triage This issue needs triage, hasn't been looked at by a team member yet label Sep 15, 2021
@osi
Copy link
Contributor Author

osi commented Sep 15, 2021

A workaround appears to be to use

Sinks.many().replay().limit(Duration.ZERO)

@simonbasle simonbasle added type/bug A general bug warn/regression A regression from a previous release status/has-workaround This has a known workaround described and removed ❓need-triage This issue needs triage, hasn't been looked at by a team member yet labels Sep 15, 2021
@simonbasle
Copy link
Member

@osi do you feel the workaround could be the recommended way of achieving this ?
Duration.ZERO is a bit more logical-sounding than limit=0, maybe?

Otherwise we can look into reintroducing support for limit = 0.

cc @OlegDokuka

@simonbasle simonbasle added this to the 3.4.11 milestone Sep 15, 2021
@osi
Copy link
Contributor Author

osi commented Sep 15, 2021

I'm fine with the workaround being the recommended way of achieving this.

More explicit support for this would be nice, but also not required.

(I use this construct in some of my test code when simulating external services)

@simonbasle simonbasle removed the warn/regression A regression from a previous release label Sep 20, 2021
@simonbasle
Copy link
Member

so as a first remediation, we could just document this workaround in the javadoc?

if more production-like usecases surface later on we can consider adding a sink implementation that re-enables each usecase.

@osi
Copy link
Contributor Author

osi commented Sep 20, 2021

Yes, I think documentation sufficient for now, and javadoc feels like an appropriate place for it.

@simonbasle simonbasle added type/documentation A documentation update and removed type/bug A general bug labels Sep 22, 2021
simonbasle added a commit that referenced this issue Sep 24, 2021
This commit documents `limit(Duration.ZERO)` as a possible alternative
to `Sinks.many().replay().limit(0)`, which is forbidden.

Fixes #2772.
simonbasle added a commit that referenced this issue Sep 24, 2021
This commit documents `limit(Duration.ZERO)` as a possible alternative
to `Sinks.many().replay().limit(0)`, which is forbidden.

Fixes #2772.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/has-workaround This has a known workaround described type/documentation A documentation update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants