Skip to content

Temporary fix for #1138 #1140

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

Merged
merged 1 commit into from
Mar 30, 2020

Conversation

yangzhe1990
Copy link
Contributor

@yangzhe1990 yangzhe1990 commented Mar 27, 2020

When GetBlockTxn is timed out, do not send the request again, instead send GetBlocks request.

Some namings are changed along with this PR.


This change is Reviewable

Copy link
Contributor

@sparkmiw sparkmiw left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 9 files at r1.
Reviewable status: 2 of 9 files reviewed, 5 unresolved discussions (waiting on @yangzhe1990)


core/src/sync/synchronization_protocol_handler.rs, line 1421 at r1 (raw file):

    pub fn blocks_received(
        &self, io: &dyn NetworkContext,
        requested_hashes_except_blah: HashSet<H256>,

what does "blah" mean?

Copy link
Contributor

@sparkmiw sparkmiw left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 9 files at r1.
Reviewable status: 3 of 9 files reviewed, 6 unresolved discussions (waiting on @yangzhe1990)


core/src/sync/message/get_block_txn.rs, line 55 at r1 (raw file):

            // where GetBlockTxn can not be initiated.
            with_public: false,
            hashes: vec![self.block_hash.clone()],

why self.clone() is not ok?

Copy link
Contributor

@sparkmiw sparkmiw left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 9 files at r1.
Reviewable status: 8 of 9 files reviewed, 7 unresolved discussions (waiting on @yangzhe1990)


core/src/sync/request_manager/mod.rs, line 661 at r1 (raw file):

    pub fn blocks_received(
        &self, io: &dyn NetworkContext,
        requested_hashes_except_blah: HashSet<H256>,

same as above

Copy link
Contributor

@sparkmiw sparkmiw left a comment

Choose a reason for hiding this comment

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

Reviewable status: 8 of 9 files reviewed, 7 unresolved discussions (waiting on @yangzhe1990)


core/src/sync/message/get_block_txn.rs, line 55 at r1 (raw file):

Previously, sparkmiw wrote…

why self.clone() is not ok?

oh, I got it.

Copy link
Contributor

@sparkmiw sparkmiw left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 9 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @yangzhe1990)

@yangzhe1990 yangzhe1990 force-pushed the refactor_send_request branch from 591db6c to 037bbd0 Compare March 30, 2020 09:36
…do not send the request again, instead

send GetBlocks request.
@yangzhe1990 yangzhe1990 force-pushed the refactor_send_request branch from 037bbd0 to 23b2719 Compare March 30, 2020 09:38
@yangzhe1990 yangzhe1990 requested a review from sparkmiw March 30, 2020 09:40
Copy link
Contributor Author

@yangzhe1990 yangzhe1990 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 4 of 8 files reviewed, 2 unresolved discussions (waiting on @sparkmiw)


core/src/sync/synchronization_protocol_handler.rs, line 1421 at r1 (raw file):

Previously, sparkmiw wrote…

what does "blah" mean?

Done.


core/src/sync/request_manager/mod.rs, line 661 at r1 (raw file):

Previously, sparkmiw wrote…

same as above

Done.

Copy link
Contributor

@sparkmiw sparkmiw left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 5 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@yangzhe1990 yangzhe1990 merged commit 2252845 into Conflux-Chain:master Mar 30, 2020
@yangzhe1990 yangzhe1990 deleted the refactor_send_request branch March 30, 2020 14:59
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.

inflight_keys for GET_BLOCK may become dangle when GetBlockTxn is discarded after an extremely long timeout.
3 participants