Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

[WIP] Shed subscription #1014

Closed
wants to merge 1 commit into from
Closed

[WIP] Shed subscription #1014

wants to merge 1 commit into from

Conversation

janos
Copy link
Member

@janos janos commented Nov 23, 2018

Preemptive PR only for subscription implementation.

swarm/shed/index.go Show resolved Hide resolved

// triggers are used for signaling
// subscriptions to continue iterations.
triggers map[uint64]chan struct{}
Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member

@zelig zelig Nov 25, 2018

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 on Put and Iterate. In this case the Index API would have SubscribeToPrefix 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 end

that 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 the from.
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)

swarm/shed/index.go Show resolved Hide resolved
@zelig
Copy link
Member

zelig commented Nov 26, 2018

summary of discussions with @janos and @frncmx

  • the batch write logic of the db is misguided and unnecessary

  • subscriptions are also unnecessary [WIP] Shed subscription #1014

  • the multimodal accessor pattern is probably a good approach, to be impemented in localstore together with triggers and iterators, see Implement DB store API #990

  • subscriptions should have an easy interface for per-bin pull sync streams

  • garbage collection should be implemented within localstore

  • push sync should be using iterations reset after RetryInterval passes [WIP] Push syncing pss  #971

  • we explored the idea to simplify push and pull sync - ideally pull sync can be simplified and use only live mode. Maybe the streams will also not need this feature

  • we discussed the idea of composite indexes in shed where keys would be of the form <prefix>|<key>|<suffix_i> -> <value_i> for 0 < i < n for a n-ary composite index .
    In particular Hash->StoreTimestamp|ChunkData and Hash->AccessTimestamp composite index could solve both issue of 1) no independent Get lookup for data and access timestamp and 2) no spurious write of chunk data when access timestamp is updated

@zelig
Copy link
Member

zelig commented Jan 8, 2019

obsoleted by #1032

@zelig zelig closed this Jan 8, 2019
@frncmx frncmx deleted the shed-subscription branch January 8, 2019 09:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants