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 Passed Passed code review & PR owner thinks it's ready to merge. 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 Report

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

Project coverage is 76.96%. Comparing base (bcbf6c1) to head (a52b05c).

Files 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 Passed Passed code review & PR owner thinks it's ready to merge. 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 2, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 2, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 2, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 2, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 2, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 3, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 4, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 5, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 5, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 5, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 6, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 6, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 6, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 8, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 11, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 11, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 11, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 11, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 11, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 11, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 25, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Sep 30, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Oct 15, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
ximinez added a commit to ximinez/rippled that referenced this pull request Oct 18, 2024
Particularly avoid acquiring ledgers likely to be
produced locally very soon.

Derived from XRPLF#4764

Co-authored-by: Mark Travis <[email protected]>
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