This repository has been archived by the owner on Aug 2, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 111
[WIP] Shed subscription #1014
Closed
Closed
[WIP] Shed subscription #1014
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a trigger should be associated with a subscription not an index.
E.g., stream pkg subscribe to bin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this code, sending and receiving part of the subscriptions are completely decoupled. Subscription only receives data and controls whether the receiving should stop or terminate. While only index is responsible for writing (putting keys and values) and signaling any subscriptions on such events. So the responsibility is completely divided. Index does not need to track every existing subscription to trigger them, but just needs to call one function, while any subscription can be created and stopped without the need to adjust any trigger invocation.
It is of course possible to have triggers on Subscription, but then, both sending and receiving parts need to be changed when a subscription is created or stopped. Which may not be the problem if we do not create subscriptions dynamically or we want to manage them outside of this package.
For subscribing to a particular bin which is a prefix in front of the timestamp in the index, we would need subscriptions that work on a prefix and to see if we want to manage this subscriptions in general way in shed package or very specific in localstore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janos as long as it is possible to have bin specific triggers i am ok with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but looking at the code i dont think that is possible. Even if trigger is created in
NewSubscription
it should be assigned as a field of the subscription so that it can be triggered by the user. you could also just give the channel as an argument. Either way it could then just be a lazy event subscription triggered (written to with a default select) if there is a new item relevant for the iteration. Otherwise how would you do this?All in all, I would eliminate the triggers map too. and provide either the channel as an argument or create it on the subscription and have a function
Trigger()
. In the latter case you would call Trigger on each bin's subscription when a new item is entered.Alternatively, we could generalise it within
shed
if we had 'prefix' hooks onPut
andIterate
. In this case the Index API would haveSubscribeToPrefix
which would subscribe to Put with that prefix. The advantage of this is that performancewise the subscription need to be created only if one iterator reaches the endthat said it would not be the same for every case. For instance garbage collection as well as push sync could be implemented with triggers. under this model, if the iterator function returns stop==true that would put the iterator in a state waiting for the trigger. If trigger was a
chan IndexItem
we can use trigger to reset thefrom
.With this we could manage
manage 1) pull sync intervals, 2) garbage collection (stop after GCcount items, then wait till capacity reached and always start from the beginning) and 3) push sync (stop after RetryInterval and start from the beginning)