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

fix: Fix collaboration issues between two protocol #2075

Merged
merged 2 commits into from
May 20, 2020

Conversation

driftluo
Copy link
Collaborator

relayer and sync have some known problems in the critical zone of switching, which are inherent to the request/response architecture.

Previous attempts to fix the problem by allowing duplicate downloads, but this solution results in wasted network resources and slowed sync speed.

This typical problem can be solved by the node itself, and the current PR gives a temporary solution to try to check the orphan pool with a very low-frequency timer.

@driftluo driftluo requested review from a team and xxuejie May 14, 2020 04:11
@driftluo driftluo force-pushed the fix-sync-relayer-collaboration branch 2 times, most recently from dcc4ce8 to 45689f3 Compare May 14, 2020 05:29
@driftluo driftluo requested review from doitian and quake May 14, 2020 07:22
doitian
doitian previously approved these changes May 14, 2020
@doitian doitian added this to 👀 Awaiting review in CKB - Pull Requests May 18, 2020
&self.chain,
&self.shared.active_chain().tip_header().hash(),
)
}),
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/nervosnetwork/p2p/blob/master/src/protocol_handle_stream.rs#L197
p2p already issuing notify handle as a blocking call, so I think tokio::task::block_in_place( is unnecessary, also the TX_PROPOSAL_TOKEN handle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://github.com/nervosnetwork/ckb/blob/develop/ckb-bin/src/subcommand/run.rs#L91
For performance and control, p2p blocking is disabled at the ckb level and it is controlled by itself

@doitian doitian added urgent Has an upcoming deadline s:backport-needed labels May 18, 2020
CKB - Pull Requests automation moved this from 👀 Awaiting review to ✅ Reviewer approved May 19, 2020
@doitian
Copy link
Member

doitian commented May 19, 2020

@zhangsoledad Do you still have any concern about this PR?

@doitian
Copy link
Member

doitian commented May 20, 2020

bors r=quake,doitian

@bors
Copy link
Contributor

bors bot commented May 20, 2020

Build succeeded:

  • continuous-integration/travis-ci/push

@bors bors bot merged commit e12086d into nervosnetwork:develop May 20, 2020
CKB - Pull Requests automation moved this from ✅ Reviewer approved to Done May 20, 2020
@driftluo driftluo deleted the fix-sync-relayer-collaboration branch May 20, 2020 03:12
doitian pushed a commit that referenced this pull request May 21, 2020
2075: fix: Fix collaboration issues between two protocol r=quake,doitian a=driftluo

relayer and sync have some known problems in the critical zone of switching, which are inherent to the request/response architecture.

Previous attempts to fix the problem by allowing duplicate downloads, but this solution results in wasted network resources and slowed sync speed.

This typical problem can be solved by the node itself, and the current PR gives a temporary solution to try to check the orphan pool with a very low-frequency timer.



Co-authored-by: driftluo <driftluo@foxmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:backport-needed urgent Has an upcoming deadline
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants