Add exclusive file locks to Wallet, Chain and Peer db’s #2403
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR adds an exclusive file locking mechanism to the wallet databases, the chain storage LMDB database and the Peer database. This is done to prevent two processes from simultaneously accessing this databases which can cause data corruption. This was done using the fs2 crate which contains a cross platform method for acquiring an OS level write lock on a file. For all these database the first step in initialization is to attempt to acquire a firelock on the lock file in the path where the database is located. If the file is not locked then a lock is acquired and the db initialisation proceeds as normal. If the file cannot be locked because the lock is already held by another instance an error is thrown. If the process holding the lock panics the OS level lock is lost so it is not needed to clean up the file explicitly for this method to work. During testing this mechanism revealed that the wallet services were not being explicitly shutdown properly, we also plumb the shutdown signals into the services and their sub protocols. The `test_store_and_forward_send_tx` test was being particularly flakey in the PR so I have opted to ignore it for now and come back and examine it properly in the future. How was this tested: Tests were written for the wallet and chain storage db’s but it was not trivial to test the peer_manager addition as it is located in the p2p initialiser. The code was manually tested on: Mac OS X Windows 10 Ubuntu 20.01 Android
|
Can confirm it works on iOS sim and device as well. |
stringhandler
approved these changes
Nov 2, 2020
stringhandler
added a commit
that referenced
this pull request
Nov 24, 2020
Major changes since v0.6.0 --- - [#2431](#2431) Database refactor to remove checkpoints and more efficient calculation of MMR roots - [#2352](#2352) Add DNS seed support in base node Minor changes since v0.6.0 --- - [#2448](#2448) Add OpenSSL to Windows install, runtime - [#2434](#2434) Add supervisord setup notes - [#2439](#2439) Provide initial sync status to merge mining proxy - [#2377](#2377) Message malleability detect and ban - [#2447](#2447) Fix for xmrig powershell script - [#2440](#2440) Stagenet Setup Guide Corrections - [#2444](#2444) DHT connectivity waits for comms connectivity before starting - [#2427](#2427) Update merge mining runtime, README, Win install - [#2420](#2420) Show base node chain tip and sync status in the console wallet - [#2419](#2419) Add optional fee_per_gram field to console wallet Send Tx form - [#2421](#2421) Fix QR code rendering in the console wallet - [#2423](#2423) Plumb in the balance in the console wallet - [#2415](#2415) Prevent loop in peer sync by storing all peers attempted - [#2383](#2383) Implement daemon-mode in tari_base_node - [#2407](#2407) Simplify automated stress test - [#2397](#2397) Fix preset config files - [#2400](#2400) Implement wallet base node service - [#2403](#2403) Add exclusive file locks to Wallet, Chain and Peer db’s - [#2408](#2408) Fix wallet conversion error for a valid tx status - [#2356](#2356) Combine validation code to use same function in pruned and archive mode. - [#2371](#2371) Add configurable BN service request timeouts - [#2430](#2430) Implement entry and persistence of custom base node in console wallet
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds an exclusive file locking mechanism to the wallet databases, the chain storage LMDB database and the Peer database. This is done to prevent two processes from simultaneously accessing this databases which can cause data corruption.
This was done using the fs2 crate which contains a cross platform method for acquiring an OS level write lock on a file. For all these database the first step in initialization is to attempt to acquire a firelock on the lock file in the path where the database is located. If the file is not locked then a lock is acquired and the db initialisation proceeds as normal. If the file cannot be locked because the lock is already held by another instance an error is thrown. If the process holding the lock panics the OS level lock is lost so it is not needed to clean up the file explicitly for this method to work.
During testing this mechanism revealed that the wallet services were not being explicitly shutdown properly, we also plumb the shutdown signals into the services and their sub protocols.
The
test_store_and_forward_send_txtest was being particularly flakey in the PR so I have opted to ignore it for now and come back and examine it properly in the future.How Has This Been Tested?
Tests were written for the wallet and chain storage db’s but it was not trivial to test the peer_manager addition as it is located in the p2p initialiser.
The code was manually tested on:
Mac OS X
Windows 10
Ubuntu 20.01
Android
Types of changes
Checklist:
developmentbranch.cargo-fmt --allbefore pushing.cargo testsuccessfully before submitting my PR.