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.

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

Optimize when to acquire ledgers from the network. #4764

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mtrippled
Copy link
Collaborator

Particulary avoid acquiring ledgers likely to be
produced locally very soon.

High Level Overview of Change

Peer ledger acquisition is expensive. This reduces peer ledger aquisition.

Context of Change

Existing code attempts to acquire ledgers if a validation for a new ledger is received, even if it comes moments before we have created the same ledger through normal consensus processing. A peer only needs to be slightly behind the first validator for this to be triggered. This essentially causes all nodes to have expensive background ledgerData acquisition jobs to be running. The fix is to not try to acquire ledgers that are very likely to be created locally through normal processing.

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)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Performance
  • Release

* When the following are all true, it is very likely that we will
* soon validate the ledger ourselves. Therefore, avoid acquiring
* ledgers from the network if:
* + Our mode is "full". It is very likely that we will acquire
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make more sense to say "we will build the ledger" instead of "we will acquire the ledger"?

Particulary avoid acquiring ledgers likely to be
produced locally very soon.
@mtrippled mtrippled 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 Nov 29, 2023
@intelliot intelliot added Perf Attn Needed Attention needed from RippleX Performance Team and removed Performance/Resource Improvement labels Jan 10, 2024
@intelliot intelliot modified the milestones: TPS, 2.2.0 (Apr 2024) Jan 10, 2024
@intelliot intelliot added Perf Test Desired (Optional) RippleX Perf Team should look at this PR. The PR will not necessarily wait for testing to finish and removed Perf Attn Needed Attention needed from RippleX Performance Team labels Jan 25, 2024
@intelliot
Copy link
Collaborator

@mtrippled to confirm that this should be put on hold for now

@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 52.38095% with 10 lines in your changes missing coverage. Please review.

Project coverage is 76.96%. Comparing base (bcbf6c1) to head (a52b05c).
Report is 149 commits behind head on develop.

Files with missing lines Patch % Lines
src/ripple/app/ledger/impl/InboundLedgers.cpp 61.53% 0 Missing and 5 partials ⚠️
src/ripple/app/misc/NetworkOPs.cpp 37.50% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4764      +/-   ##
===========================================
- Coverage    76.97%   76.96%   -0.01%     
===========================================
  Files         1129     1129              
  Lines       131916   131932      +16     
  Branches     39614    39662      +48     
===========================================
+ Hits        101537   101542       +5     
- Misses       24435    24456      +21     
+ Partials      5944     5934      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ximinez ximinez removed Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. Blocked labels Apr 8, 2024
@intelliot intelliot modified the milestones: 2.2.0 (June 2024), TPS May 31, 2024
ximinez added a commit to ximinez/rippled that referenced this pull request Aug 16, 2024
Particulary avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Aug 17, 2024
Particulary avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Aug 17, 2024
Particulary avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Aug 23, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Aug 23, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 1, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 5, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 8, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 13, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 13, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Nov 27, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Dec 5, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Dec 16, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Dec 20, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Jan 7, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Jan 9, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Jan 25, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Jan 28, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Jan 30, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 4, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 6, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 7, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 8, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 11, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 12, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 13, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 15, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 20, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 25, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 27, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 28, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Feb 28, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 11, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 12, 2025
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <mtravis@ripple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Perf Test Desired (Optional) RippleX Perf Team should look at this PR. The PR will not necessarily wait for testing to finish
Projects
Status: 👀 Needs review
Development

Successfully merging this pull request may close these issues.

5 participants