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

Mutex scope #4063

Closed
wants to merge 7 commits into from
Closed

Mutex scope #4063

wants to merge 7 commits into from

Conversation

seelabs
Copy link
Collaborator

@seelabs seelabs commented Jan 11, 2022

As written, each thread has it's own mutex and that mutex is a dangling
reference. This patch moves the mutex outside the loop so all the threads share
a single mutex.

JoelKatz and others added 7 commits January 7, 2022 04:41
1) Don't acquire so many nodes per pass. It's likely
far more than we need.

2) Right-size the finishedReads_ vector on passes other
than just the first.
- Only duplicate records from archive to writable during online_delete.
- Log duration of nodestore reads.
- Include nodestore counters in perf_log output.
- Remove gratuitous nodestore activity counting.
- Report initial sync duration in server_info and perfLog.
- Report state_accounting in perfLog.
- Make state_accounting durations more accurate.
- Parallel ledger loader.
- Config parameter to load ledgers on start.
The nodestore includes a built-in cache to reduce the disk I/O
load but, by default, this cache was not initialized unless it
was explicitly configured by the server operator.

This commit introduces sensible defaults based on the server's
configured node size.

It remains possible to completely disable the cache if desired
by explicitly configuring it the cache size and age parameters
to 0:

    [node_db]
    ...
    cache_size = 0
    cache_age = 0
As written, each thread has it's own mutex and that mutex is a dangling
reference. This patch moves the mutex outside the loop so all the threads share
a single mutex.
Copy link
Collaborator

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

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

Mutex change looks right to me. 👍

Copy link
Collaborator

@mtrippled mtrippled left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@seelabs seelabs added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Jan 12, 2022
@nbougalis
Copy link
Contributor

Included in #4061

@nbougalis nbougalis closed this Jan 12, 2022
@nbougalis nbougalis mentioned this pull request Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants