Skip to content

fix: prevent messages from taking too long to be sent - #902

Open
ChaoticTempest wants to merge 8 commits into
developfrom
phuong/fix/prevent-msg-taking-too-long
Open

fix: prevent messages from taking too long to be sent#902
ChaoticTempest wants to merge 8 commits into
developfrom
phuong/fix/prevent-msg-taking-too-long

Conversation

@ChaoticTempest

Copy link
Copy Markdown
Contributor

messages will occasionally take over 1s to send, which can stall the node from doing work. This PR makes all the send_encrypted be ran concurrently such that the node don't wait until the previous send_encrypted gets completed. I've seen the message sending portion go from 1-2s max to 300ms max so this should be a good improvement with no breaking changes

Comment thread chain-signatures/node/src/protocol/mod.rs
Comment thread chain-signatures/node/src/http_client.rs Outdated
ailisp
ailisp previously approved these changes Oct 18, 2024

@ailisp ailisp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice improvement by concurrently sending messages!

ppca
ppca previously approved these changes Oct 18, 2024

@ppca ppca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM except the tracing comment.

Comment thread chain-signatures/node/src/protocol/mod.rs
Comment thread chain-signatures/node/src/http_client.rs Outdated
@ChaoticTempest
ChaoticTempest dismissed stale reviews from ppca and ailisp via d8d894b October 19, 2024 07:42
volovyks
volovyks previously approved these changes Oct 21, 2024

@volovyks volovyks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

Comment thread chain-signatures/node/src/protocol/mod.rs
Comment thread chain-signatures/node/src/http_client.rs
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.

4 participants