Skip to content

Replay network subscriptions on startup#33

Merged
reinkrul merged 1 commit into
masterfrom
replay-subscribers-on-startup
Feb 4, 2021
Merged

Replay network subscriptions on startup#33
reinkrul merged 1 commit into
masterfrom
replay-subscribers-on-startup

Conversation

@reinkrul

@reinkrul reinkrul commented Feb 1, 2021

Copy link
Copy Markdown
Member

Since the current DIDStore is an in-memory implementation, the DID Documents stored in Nuts Network must be loaded into it (through the subscribers) every time the node is started. This actually makes things much simpler: subscriptions don't have to be persistent, meaning that we can replay the DAG every time a new subscription is added.

This PR alters the DAG subscription handling such that it uses an altered version of the Bread-First DAG walker, one which keeps track of which documents couldn't be processed at that point: e.g. when the payload is missing, meaning we can't call the subscribers for that particular document AND all subsequent documents on that branch. When we receive payload we can call the walker again, resuming at the documents we previously stopped.

Fixes #8

@codecov

codecov Bot commented Feb 1, 2021

Copy link
Copy Markdown

Codecov Report

Merging #33 (fb6a336) into master (f6055ab) will increase coverage by 0.63%.
The diff coverage is 85.58%.

Impacted Files Coverage Δ
network/network.go 74.35% <75.00%> (+0.74%) ⬆️
network/dag/bbolt_payloadstore.go 78.94% <78.94%> (ø)
network/dag/bfswalker.go 84.84% <84.84%> (ø)
network/dag/publisher.go 87.17% <87.17%> (ø)
network/dag/bbolt_dag.go 77.19% <100.00%> (ø)

@reinkrul
reinkrul force-pushed the replay-subscribers-on-startup branch 2 times, most recently from ba60ae9 to 59fd786 Compare February 2, 2021 11:41
@reinkrul
reinkrul marked this pull request as ready for review February 2, 2021 11:47

@woutslakhorst woutslakhorst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need an issue or conversation to keep track of the architecture of dagDocument to VDR/VCR

Comment thread network/dag/publisher.go
}

func (s *replayingDAGPublisher) PayloadWritten(_ interface{}) {
s.publish(hash.EmptyHash())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the empty hash?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because when the node receives payload, we'd have to look up in the payload hash -> document index where we should resume (and check whether that's a resumation point) but that's not accessible from this type (it's an internal of the BBolt DAG implementation). So I chose to just resume from the resumation points regardless whether we can actually go forward with publishing to subscribers (it can only resume when the payload's there)

When we receive payload chances are pretty big that its from a resumation point, especially when your node is in sync with other nodes. We could optimize this later if it turns out to be a bottleneck.

@reinkrul

reinkrul commented Feb 4, 2021

Copy link
Copy Markdown
Member Author

We also need an issue or conversation to keep track of the architecture of dagDocument to VDR/VCR

I'll draw something up, made an issue just to be sure;

#47

@woutslakhorst woutslakhorst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you ignore/approve issues in codeclimate before merging?

@reinkrul
reinkrul force-pushed the replay-subscribers-on-startup branch from 59fd786 to fb6a336 Compare February 4, 2021 13:34
@reinkrul
reinkrul merged commit 05d0a34 into master Feb 4, 2021
@reinkrul
reinkrul deleted the replay-subscribers-on-startup branch February 4, 2021 13:36
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.

Improve network pub-sub

2 participants