8000 feat: improve squelching configuration by Tapanito · Pull Request #5438 · XRPLF/rippled · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

feat: improve squelching configuration #5438

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
Merged
merged 18 commits into from
May 28, 2025
Merged

feat: improve squelching configuration #5438

merged 18 commits into from
May 28, 2025

Conversation

Tapanito
Copy link
Collaborator
@Tapanito Tapanito commented May 20, 2025 •
edited
Loading

This PR introduces the following changes:

  1. Renames vp_enable config option to vp_base_squelch_enable to enable squelching for validators.
  2. Removes vp_squelch config option which was used to configure whether to send squelch messages to peers or not. With this flag removed, if squelching is enabled, squelch messages will be sent. This was an option used for debugging.
  3. Introduces a temporary vp_base_squelch_max_trusted_peers config option to change the max number of peers who are selected as validator message sources. This is a temporary option, which will be removed once a good value is found.
  4. Adds a traffic counter to count the number of times peers ignored squelch messages and kept sending messages for squelched validators.
  5. Moves the decision whether squelching is enabled and ready into Slot,h

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@Tapanito Tapanito marked this pull request as ready for review May 20, 2025 11:54
@Tapanito Tapanito requested review from bthomee and vlntb May 20, 2025 11:54
@codecov Codecov
Copy link
codecov bot commented May 20, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 67.50000% with 26 lines in your changes missing coverage. Please review.

Project coverage is 78.6%. Comparing base (be668ee) to head (c90fde0).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/overlay/detail/OverlayImpl.cpp 7.7% 12 Missing ⚠️
src/xrpld/overlay/detail/PeerImp.h 0.0% 8 Missing ⚠️
src/xrpld/overlay/detail/PeerImp.cpp 66.7% 4 Missing ⚠️
src/xrpld/overlay/detail/ConnectAttempt.cpp 0.0% 1 Missing ⚠️
src/xrpld/overlay/detail/TrafficCount.h 50.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5438     +/-   ##
=========================================
- Coverage     78.6%   78.6%   -0.0%     
=========================================
  Files          816     816             
  Lines        70506   70534     +28     
  Branches      8287    8287             
=========================================
+ Hits         55452   55469     +17     
- Misses       15054   15065     +11
Files with missing lines Coverage Δ
src/xrpld/core/Config.h 85.7% <ø> (ø)
src/xrpld/core/detail/Config.cpp 75.8% <100.0%> (+0.6%) ⬆️
src/xrpld/overlay/Slot.h 84.8% <100.0%> (+1.1%) ⬆️
src/xrpld/overlay/detail/Handshake.cpp 50.8% <100.0%> (ø)
src/xrpld/overlay/detail/Handshake.h 100.0% <ø> (ø)
src/xrpld/overlay/detail/ConnectAttempt.cpp 0.0% <0.0%> (ø)
src/xrpld/overlay/detail/TrafficCount.h 60.0% <50.0%> (-0.1%) ⬇️
src/xrpld/overlay/detail/PeerImp.cpp 3.7% <66.7%> (+<0.1%) ⬆️
src/xrpld/overlay/detail/PeerImp.h 13.0% <0.0%> (ø)
src/xrpld/overlay/detail/OverlayImpl.cpp 34.8% <7.7%> (-0.3%) ⬇️

... and 6 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Tapanito Tapanito force-pushed the tapanito/feature/squelching-targets-config branch from 728a824 to 8a8c804 Compare May 22, 2025 08:03
bthomee
bthomee reviewed May 22, 2025
View reviewed changes
@Tapanito Tapanito force-pushed the tapanito/feature/squelching-targets-config branch from d0fed82 to ac3b1dd Compare May 22, 2025 17:46
bthomee
bthomee reviewed May 22, 2025
View reviewed changes
@Tapanito Tapanito force-pushed the tapanito/feature/squelching-targets-config branch from 9dfb68f to 5be2e9e Compare May 22, 2025 21:46
bthomee
bthomee reviewed May 23, 2025
View reviewed changes
bthomee
bthomee approved these changes May 23, 2025
View reviewed changes
vlntb
vlntb reviewed May 27, 2025
View reviewed changes
vlntb
vlntb reviewed May 27, 2025
View reviewed changes
vlntb
vlntb reviewed May 27, 2025
View reviewed changes
vlntb
vlntb requested changes May 27, 2025
View reviewed changes
Copy link
Collaborator
@vlntb vlntb left a comment

Choose a reason for hiding this comment

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

Please see my 3 comments in the PR.

@Tapanito
Copy link
Collaborator Author

Thank you for reviewing the code @vlntb 🙇 . I addressed your comments.

@bthomee bthomee added this to the 2.5.0 (Q2 2025) milestone May 27, 2025
@bthomee bthomee requested a review from vlntb May 27, 2025 20:34
vlntb
vlntb approved these changes May 28, 2025
View reviewed changes
Copy link
Collaborator
@vlntb vlntb left a comment

Choose a reason for hiding this comment

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

Everything is addressed and looks good!

@Tapanito Tapanito added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 28, 2025
@bthomee bthomee merged commit d71ce51 into develop May 28, 2025
27 of 28 checks passed
@bthomee bthomee deleted the tapanito/feature/squelching-targets-config branch May 28, 2025 10:30
This was referenced Jun 12, 2025
Merged
Closed
Closed
@legleux legleux mentioned this pull request Jun 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bthomee bthomee bthomee approved these changes

@vlntb vlntb vlntb approved these changes

Assignees
No one assigned
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Milestone
2.5.0 (Q2 2025)
Development

Successfully merging this pull request may close these issues.
3 participants
@Tapanito @bthomee @vlntb
0