8000 Remove lambda from `rotateWithLock` by Bronek · Pull Request #5286 · XRPLF/rippled · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove lambda from rotateWithLock #5286

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

Closed
wants to merge 2 commits into from
Closed

Conversation

Bronek
Copy link
Collaborator
@Bronek Bronek commented Feb 12, 2025

Possible improvement to PR #5276

This is based on assumption that we do not need to call clearCaches(validatedSeq) twice. There should be no reason to call it second time inside the critical section, just like there should be no reason to update state_db_ inside the critical section, so this change minimises the critical section and also removes the risk that some untested code from outside of DatabaseRotatingImp could run inside it.

@Bronek
Copy link
Collaborator Author
Bronek commented Feb 12, 2025

I would add levelization fix to this PR but the branch protection rules forbid me from adding any commits :-|

Copy link
codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 68.75000% with 5 lines in your changes missing coverage. Please review.

Please upload report for BASE (ximinez/db-lock@217bc1d). Learn more about missing BASE report.

Files with missing lines Patch % Lines
s 8000 rc/xrpld/nodestore/detail/DatabaseRotatingImp.cpp 54.5% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##             ximinez/db-lock   #5286   +/-   ##
=================================================
  Coverage                   ?   78.2%           
=================================================
  Files                      ?     790           
  Lines                      ?   67634           
  Branches                   ?    8162           
=================================================
  Hits                       ?   52858           
  Misses                     ?   14776           
  Partials                   ?       0           
Files with missing lines Coverage Δ
src/xrpld/app/misc/SHAMapStoreImp.cpp 76.8% <100.0%> (ø)
src/xrpld/nodestore/DatabaseRotating.h 100.0% <ø> (ø)
src/xrpld/nodestore/detail/DatabaseRotatingImp.h 66.7% <ø> (ø)
src/xrpld/nodestore/detail/DatabaseRotatingImp.cpp 61.2% <54.5%> (ø)

Impacted file tree graph

@Bronek
Copy link
Collaborator Author
Bronek commented Feb 12, 2025

Judging by comments in #3342 it seems that we do need both clearCaches and update state_db_ inside the critical section. :-(

@Bronek Bronek closed this Feb 13, 2025
@Bronek Bronek deleted the bronek/db-lock branch June 10, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant
0