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

KeepAlive Client collect samples on first packet #4693

Merged
merged 1 commit into from
Oct 27, 2023
Merged

KeepAlive Client collect samples on first packet #4693

merged 1 commit into from
Oct 27, 2023

Conversation

karknu
Copy link
Contributor

@karknu karknu commented Oct 17, 2023

Description

Update the KeepAlive client so that it collect RTT samples for the first packet. Previusly two request where sent, one after the other because the first couldn't be used to get a sample.
Fixes #4689

Checklist

  • Branch
    • Updated changelog files.
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • The documentation has been properly updated
    • New tests are added if needed and existing tests are updated
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@karknu karknu requested a review from coot as a code owner October 17, 2023 08:46
coot
coot reviewed Oct 17, 2023
View reviewed changes
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor remarks.

ouroboros-network-protocols/src/Ouroboros/Network/Protocol/KeepAlive/Client.hs Outdated Show resolved Hide resolved
ouroboros-network-protocols/testlib/Ouroboros/Network/Protocol/KeepAlive/Direct.hs Outdated Show resolved Hide resolved
ouroboros-network-protocols/testlib/Ouroboros/Network/Protocol/KeepAlive/Direct.hs Show resolved Hide resolved
@karknu karknu force-pushed the karknu/keepalive_start branch 2 times, most recently from 049bab9 to 1a04523 Compare October 18, 2023 06:19
coot
coot approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

@karknu
KeepAlive Client collect samples on first packet
1728830 
Update the KeepAlive client so that it collect RTT samples for the first
packet. Previusly two request where sent, one after the other because
the first couldn't be used to get a sample.
@karknu karknu force-pushed the karknu/keepalive_start branch from 1a04523 to 1728830 Compare October 27, 2023 09:15
coot
coot approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

@coot coot enabled auto-merge October 27, 2023 09:20
@coot coot added this pull request to the merge queue Oct 27, 2023
Merged via the queue into IntersectMBO:master with commit 80e823a Oct 27, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coot coot coot approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

KeepAlive client
2 participants
@karknu @coot