Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: prevent redundant blocksync block messages #4909

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

SamHSmith
Copy link
Contributor

@SamHSmith SamHSmith commented Jul 27, 2024

Description

There was a problem during TPS testing where block sync could not occur quickly because it would re-request the
same blocks from each peer. Here is a solution that should be robust against that scenario but doesn't introduce
any new issues.

Linked issue

Closes #{issue_number}

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

Copy link
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

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

I think this PR is step in the right direction, minor fixes and i would approve.

core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Show resolved Hide resolved
core/src/block_sync.rs Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
@mversic mversic self-assigned this Jul 30, 2024
@mversic
Copy link
Contributor

mversic commented Jul 30, 2024

this should be backported to rc22

@SamHSmith SamHSmith force-pushed the smart_block_sync branch 3 times, most recently from 91eb3a1 to fcafd58 Compare July 31, 2024 00:23
@mversic
Copy link
Contributor

mversic commented Aug 5, 2024

you have a failing test in torii api

@SamHSmith SamHSmith force-pushed the smart_block_sync branch 3 times, most recently from 068c470 to 8f64585 Compare August 6, 2024 16:53
@SamHSmith
Copy link
Contributor Author

you have a failing test in torii api

It has now been fixed.

Erigara
Erigara previously approved these changes Aug 7, 2024
mversic
mversic previously approved these changes Aug 8, 2024
core/src/block_sync.rs Outdated Show resolved Hide resolved
core/src/block_sync.rs Outdated Show resolved Hide resolved
@mversic mversic self-requested a review August 8, 2024 07:13
core/src/block_sync.rs Outdated Show resolved Hide resolved
@SamHSmith SamHSmith force-pushed the smart_block_sync branch 2 times, most recently from fa756e9 to 9b46f17 Compare August 8, 2024 09:44
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
@mversic mversic merged commit 9e8e214 into hyperledger-iroha:main Aug 8, 2024
11 of 12 checks passed
mversic pushed a commit that referenced this pull request Aug 30, 2024
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants