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.

Sign up for GitHub

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

Jump to bottom

delayed pubsub subscribe for messages topic #3646

Merged
merged 5 commits into from
Nov 13, 2020
Merged

delayed pubsub subscribe for messages topic #3646

merged 5 commits into from
Nov 13, 2020

Conversation

vyzo
Copy link
Contributor

@vyzo vyzo commented Sep 8, 2020 •
edited
Loading

Reinstates the code that got reverted, but with a couple of improvements:

  • we only delay subscription to the messages topic, so that we avoid the debacle with failed syncing.
  • we wait to be within 10 epochs before subscribing instead of the overly aggressive 1.

Also moves the subscription code in blocks after the validator registration, which is the correct order of things.

Please test in your nodes!

@vyzo
Copy link
Contributor Author

vyzo commented Sep 8, 2020

fixed bug that prevented us from joining; block timestamp is in seconds, time was checked in nanoseconds!

@vyzo vyzo mentioned this pull request Sep 8, 2020
Merged
@vyzo vyzo force-pushed the feat/delayed-pubsub-subscribe branch 2 times, most recently from 2df732f to 32f2a3e Compare September 8, 2020 15:49
@vyzo
Copy link
Contributor Author

vyzo commented Sep 8, 2020

rebased on master.

@vyzo vyzo force-pushed the feat/delayed-pubsub-subscribe branch from 32f2a3e to 46bd1ed Compare September 10, 2020 13:22
@vyzo
Copy link
Contributor Author

vyzo commented Sep 10, 2020

rebased on master and added variable controlling sync epochs so that user can override the magic number 10.

@magik6k
Copy link
Contributor

magik6k commented Oct 19, 2020

Fixes #469

arajasek
arajasek approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

LGTM, but def want review from some of @magik6k / @Kubuxu / @whyrusleeping

@arajasek
Copy link
Contributor

arajasek commented Nov 5, 2020

(and a rebase)

@vyzo vyzo force-pushed the feat/delayed-pubsub-subscribe branch from 46bd1ed to dab20ef Compare November 5, 2020 16:12
@vyzo
Copy link
Contributor Author

vyzo commented Nov 5, 2020

rebased on master.

@jennijuju jennijuju linked an issue Nov 6, 2020 that may be closed by this pull request
Don't subscribe to message gossip until chain is up to date #469
Closed
@arajasek arajasek merged commit 1af1ceb into master Nov 13, 2020
@arajasek arajasek deleted the feat/delayed-pubsub-subscribe branch November 13, 2020 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@travisperson travisperson travisperson approved these changes

@whyrusleeping whyrusleeping whyrusleeping approved these changes

@arajasek arajasek arajasek approved these changes

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

@magik6k magik6k Awaiting requested review from magik6k

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
🖇Lotus Stabilization (1.1.3)
Development

Successfully merging this pull request may close these issues.

Don't subscribe to message gossip until chain is up to date
6 participants
@vyzo @magik6k @arajasek @travisperson @whyrusleeping @jennijuju