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

fix node size estimation #4536

Merged
merged 2 commits into from
Jun 9, 2023
Merged

Conversation

dangell7
Copy link
Collaborator

High Level Overview of Change

This patch fixes a bug in the NODE_SIZE autodetection feature in the Config.cpp file. Specifically, this patch corrects the calculation for the total amount of RAM available, which was previously returned in bytes, but is now being returned in units of the system's memory unit. Additionally, the patch adjusts the node size based on the number of available hardware threads of execution.

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Copy link
Contributor

@nbougalis nbougalis left a comment

Choose a reason for hiding this comment

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

Looks good modulo the one minor change I listed. Once that's done, this gets a 👍 from me.

src/ripple/core/impl/Config.cpp Outdated Show resolved Hide resolved
@nbougalis nbougalis added Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. Bug labels Jun 5, 2023
@nbougalis
Copy link
Contributor

This should be included in the next release, as it fixes a bug.

@intelliot intelliot requested a review from manojsdoshi June 6, 2023 05:33
@intelliot
Copy link
Collaborator

intelliot commented Jun 6, 2023

@manojsdoshi to confirm whether this is ok to include in 1.11

Standard procedure is to get 2 approvals, unless the PR is obviously trivial (and in that case, it needs the Trivial label applied). @nbougalis - Can you recommend another reviewer?

@intelliot
Copy link
Collaborator

Perhaps @sophiax851 or @ChronusZ can take a look?

@nbougalis
Copy link
Contributor

This is a bug fix that impacts servers and, imo, is pretty trivial. Maybe @HowardHinnant can take a quick look?

@intelliot intelliot requested a review from HowardHinnant June 6, 2023 16:52
@intelliot intelliot added this to the 1.11.0 milestone Jun 8, 2023
@intelliot
Copy link
Collaborator

QA confirmed this is ok to include in 1.11 (if approved by code review)

Copy link
Contributor

@HowardHinnant HowardHinnant left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Disclaimer: Part of this change is on a Linux-only branch which I'm not able to test. However I know @nbougalis is using Linux and he's approved, so I think we're covered.

@intelliot intelliot merged commit 5d011c7 into XRPLF:develop Jun 9, 2023
@intelliot
Copy link
Collaborator

Note: This was an older bug (i.e. not a bug introduced in 1.11 nor in an RC - release candidate). It's okay this time, but in the future, QA would like to avoid making non-critical (non-blocker) changes within 2 weeks of a release.

ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Jul 10, 2023
Fix a bug in the `NODE_SIZE` auto-detection feature in `Config.cpp`.
Specifically, this patch corrects the calculation for the total amount
of RAM available, which was previously returned in bytes, but is now
being returned in units of the system's memory unit. Additionally, the
patch adjusts the node size based on the number of available hardware
threads of execution.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 22, 2023
Fix a bug in the `NODE_SIZE` auto-detection feature in `Config.cpp`.
Specifically, this patch corrects the calculation for the total amount
of RAM available, which was previously returned in bytes, but is now
being returned in units of the system's memory unit. Additionally, the
patch adjusts the node size based on the number of available hardware
threads of execution.
ckeshava pushed a commit to ckeshava/rippled that referenced this pull request Sep 25, 2023
Fix a bug in the `NODE_SIZE` auto-detection feature in `Config.cpp`.
Specifically, this patch corrects the calculation for the total amount
of RAM available, which was previously returned in bytes, but is now
being returned in units of the system's memory unit. Additionally, the
patch adjusts the node size based on the number of available hardware
threads of execution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants