Skip to content

DRAFT: Reduce duplicate peer traffic for ledger data #5301

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
Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

DRAFT: Reduce duplicate peer traffic for ledger data #5301

wants to merge 3 commits into from

Conversation

@ximinez
Copy link
Collaborator

@ximinez ximinez commented Feb 19, 2025 •
edited
Loading

Recreates the changes from #5126 (reverted in #5300), and the related bug fix once I find it.

  • Drop duplicate outgoing TMGetLedger messages per peer
    • Allow a retry after 30s in case of peer or network congestion.
    • Addresses RIPD-1870
    • (Changes levelization. That is not desirable, and will need to be fixed.)
  • Drop duplicate incoming TMGetLedger messages per peer
    • Allow a retry after 15s in case of peer or network congestion.
    • The requestCookie is ignored when computing the hash, thus increasing the chances of detecting duplicate messages.
    • With duplicate messages, keep track of the different requestCookies (or lack of cookie). When work is finally done for a given request, send the response to all the peers that are waiting on the request, sending one message per peer, including all the cookies and a "directResponse" flag indicating the data is intended for the sender, too.
    • Addresses RIPD-1871
  • Drop duplicate incoming TMLedgerData messages
    • Addresses RIPD-1869
  • Improve logging related to ledger acquisition
  • Class "CanProcess" to keep track of processing of distinct items

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)

@ximinez ximinez changed the title DRAFT: Reduce duplicate peer traffic for ledger data (#5126) DRAFT: Reduce duplicate peer traffic for ledger data Feb 19, 2025
@codecov
Copy link

codecov bot commented Feb 20, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 24.34988% with 320 lines in your changes missing coverage. Please review.

Project coverage is 77.9%. Comparing base (1c99ea2) to head (3a685d6).

Files with missing lines Patch % Lines
src/xrpld/overlay/detail/PeerImp.cpp 0.0% 210 Missing ⚠️
src/xrpld/overlay/detail/ProtocolMessage.h 0.0% 32 Missing ⚠️
src/xrpld/app/misc/NetworkOPs.cpp 21.4% 22 Missing ⚠️
src/xrpld/overlay/detail/PeerSet.cpp 20.8% 19 Missing ⚠️
src/xrpld/app/ledger/detail/InboundLedgers.cpp 67.9% 17 Missing ⚠️
src/xrpld/app/ledger/detail/InboundLedger.cpp 54.5% 5 Missing ⚠️
src/xrpld/app/ledger/InboundLedger.h 60.0% 4 Missing ⚠️
src/xrpld/app/misc/HashRouter.cpp 60.0% 4 Missing ⚠️
src/xrpld/app/ledger/detail/LedgerMaster.cpp 0.0% 3 Missing ⚠️
src/xrpld/app/misc/HashRouter.h 77.8% 2 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5301     +/-   ##
=========================================
- Coverage     78.1%   77.9%   -0.2%     
=========================================
  Files          795     796      +1     
  Lines        68598   68921    +323     
  Branches      8281    8441    +160     
=========================================
+ Hits         53584   53668     +84     
- Misses       15014   15253    +239
Files with missing lines Coverage Δ
include/xrpl/basics/base_uint.h 96.9% <100.0%> (+<0.1%) ⬆️
include/xrpl/protocol/LedgerHeader.h 100.0% <ø> (ø)
src/xrpld/app/ledger/detail/TimeoutCounter.cpp 88.4% <100.0%> (+1.3%) ⬆️
src/xrpld/app/ledger/detail/TimeoutCounter.h 100.0% <ø> (ø)
src/xrpld/app/misc/NetworkOPs.h 100.0% <ø> (ø)
src/xrpld/overlay/Peer.h 100.0% <ø> (ø)
src/xrpld/overlay/detail/PeerImp.h 12.8% <ø> (ø)
src/xrpld/overlay/detail/ProtocolVersion.cpp 86.4% <ø> (ø)
include/xrpl/basics/CanProcess.h 95.5% <95.5%> (ø)
src/xrpld/app/consensus/RCLConsensus.cpp 62.2% <0.0%> (ø)
... and 10 more

... and 5 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.

@ximinez ximinez force-pushed the ximinez/fix-getledger branch 3 times, most recently from 73497e5 to 15e4475 Compare February 27, 2025 00:06
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 2 times, most recently from 8a1331c to 021c10d Compare February 28, 2025 18:55
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 5 times, most recently from 46f8af5 to 0e4f4d2 Compare March 17, 2025 23:01
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 5 times, most recently from 0b8c2b3 to f2b1a67 Compare March 25, 2025 16:12
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 3 times, most recently from 966204c to c49d81c Compare April 2, 2025 14:43
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 7 times, most recently from 503ed03 to 3a685d6 Compare April 10, 2025 21:39
@ximinez ximinez force-pushed the ximinez/fix-getledger branch from 3a685d6 to 99fb808 Compare April 11, 2025 22:28
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 2 times, most recently from 706a4c9 to 3f71758 Compare April 25, 2025 15:46
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 5 times, most recently from 13fe701 to 67133c2 Compare September 16, 2025 16:36
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 3 times, most recently from 0df2bf1 to 8c34daf Compare September 20, 2025 19:44
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 9 times, most recently from e68b4d0 to 698a3af Compare October 1, 2025 17:14
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 2 times, most recently from 4847b19 to 717fb59 Compare October 2, 2025 15:03
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 4 times, most recently from 67f07ec to 7d53a0e Compare October 16, 2025 17:12
@ximinez ximinez force-pushed the ximinez/fix-getledger branch 3 times, most recently from 8b16d22 to f429eec Compare October 22, 2025 16:14
@ximinez
Improve job queue collision checks and logging
89121ce 
- Improve logging related to ledger acquisition and operating mode
  changes
- Class "CanProcess" to keep track of processing of distinct items
@ximinez
Reduce duplicate peer traffic for ledger data (#5126)
1dd447c 
- Drop duplicate outgoing TMGetLedger messages per peer
  - Allow a retry after 30s in case of peer or network congestion.
  - Addresses RIPD-1870
  - (Changes levelization. That is not desirable, and will need to be fixed.)
- Drop duplicate incoming TMGetLedger messages per peer
  - Allow a retry after 15s in case of peer or network congestion.
  - The requestCookie is ignored when computing the hash, thus increasing
    the chances of detecting duplicate messages.
  - With duplicate messages, keep track of the different requestCookies
    (or lack of cookie). When work is finally done for a given request,
    send the response to all the peers that are waiting on the request,
    sending one message per peer, including all the cookies and
    a "directResponse" flag indicating the data is intended for the
    sender, too.
  - Addresses RIPD-1871
- Drop duplicate incoming TMLedgerData messages
  - Addresses RIPD-1869
@ximinez
Fix formatting
1893101
@ximinez ximinez force-pushed the ximinez/fix-getledger branch from f429eec to 1893101 Compare October 23, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ximinez