Skip to content

Conversation

@wjblanke
Copy link
Contributor

@wjblanke wjblanke commented Jan 10, 2025

Purpose:

The db_wrapper code allows read operations to use the current write connection if it is active.

Currently the syncing operation uses distinct writes and reads which means write operations are on the writer connection and read operations are on one of the read connections across different threads.

To make life easier for sqlite, this PR wraps block syncs within one db_wrapper writer. The block sync db_wrapper operations then will use the write connection whether it is for reading or writing. The write connection always has an up to date view of the DB.

Current Behavior:

Block sync operations use multiple connections and threads

New Behavior:

Block sync operations use the writer connection

Testing Notes:

This may cause a reduction in concurrency because the db_wrapper write has its own lock. Performance would be the main concern here although with less coherency issues things should be faster.

@wjblanke wjblanke added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Jan 10, 2025
@wjblanke wjblanke requested a review from a team as a code owner January 10, 2025 13:17
@wjblanke wjblanke requested a review from arvidn January 10, 2025 13:17
@coveralls-official
Copy link

coveralls-official bot commented Jan 12, 2025

Pull Request Test Coverage Report for Build 12734468896

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 2 files are covered.
  • 271 unchanged lines in 16 files lost coverage.
  • Overall coverage decreased (-0.02%) to 91.535%

Files with Coverage Reduction New Missed Lines %
chia/_tests/simulation/test_simulation.py 1 96.45%
chia/daemon/keychain_proxy.py 1 73.24%
chia/full_node/pending_tx_cache.py 1 96.55%
chia/farmer/farmer.py 1 73.37%
chia/daemon/client.py 1 74.16%
chia/plotters/plotters.py 1 90.94%
chia/rpc/util.py 2 95.74%
chia/server/node_discovery.py 3 81.32%
chia/full_node/full_node_api.py 3 84.7%
chia/rpc/rpc_server.py 3 88.24%
Totals Coverage Status
Change from base Build 12691593511: -0.02%
Covered Lines: 105336
Relevant Lines: 114897

💛 - Coveralls

@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Jan 27, 2025
@github-actions
Copy link
Contributor

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

@github-actions github-actions bot added the stale-pr Flagged as stale and in need of manual review label Mar 14, 2025
@github-actions github-actions bot removed the stale-pr Flagged as stale and in need of manual review label Aug 21, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2025

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

@github-actions github-actions bot added the stale-pr Flagged as stale and in need of manual review label Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changed Required label for PR that categorizes merge commit message as "Changed" for changelog merge_conflict Branch has conflicts that prevent merge to main stale-pr Flagged as stale and in need of manual review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants