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

Implements fs2.concurrent.Topic with fs2.concurrent.Queue #1407

Closed

Conversation

vladimir-popov
Copy link

@vladimir-popov vladimir-popov commented Jan 29, 2019

Implementation of the fs2.concurrent.Topic with fs2.concurrent.Queue as subscriptions. Read more in this issue #1406 .

@pchlupacek
Copy link
Contributor

Just a quick comment on this. If we wont manage to get perfromance correct, there may be also ather combinators affected. So instead of getting this as quick solution only for topic I would like to go through pubsub, and make sure that

  • this cannot be solved with pubsub
  • if this cannot be solved with pubsub make sure that no other combinators using the pubsub (i.e. broadcast) are not affected by this too
  • fix all the other combinators, if this will be pubsub design flaw.

@SystemFw
Copy link
Collaborator

SystemFw commented Jan 29, 2019

I agree with @pchlupacek viewpoint on this: we actually already have a battle tested implementation of Topic using Queue - the pre PubSub one.

I'd rather see if we can fix it with PubSub though, cause it's much more flexible while at the same time abstracting away a lot of complicated and potentially unsafe synchronisation logic.

@vladimir-popov
Copy link
Author

vladimir-popov commented Jan 30, 2019

Ok. I understand your point. Now I’ll close this PR, but we will continue use it as temporary workaround.

@pchlupacek
Copy link
Contributor

Thats completely fine @vladimir-popov

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.

None yet

3 participants