Use LDBVersionedStore for wallet also #1011
Merged
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
aslesarenko
requested changes
Dec 25, 2019
| def get(key: K): Option[V] = { | ||
| lock.readLock().lock() | ||
| val res = Option(db.get(key)) | ||
| lock.readLock().unlock() |
There was a problem hiding this comment.
Shouldn't it be protected with try {..} finally { ... } section?
|
|
||
|
|
||
| /** | ||
| * Iterate through the database to read elements according to a filter function |
There was a problem hiding this comment.
Suggested change
| * Iterate through the database to read elements according to a filter function | |
| * Iterate through the database to read elements according to a filter function. | |
| * The method first load filtered data in memory and then returns an iterator over them. |
| } | ||
|
|
||
| /** | ||
| * Read all the database elements |
There was a problem hiding this comment.
Suggested change
| * Read all the database elements | |
| * Read all the database elements. | |
| * The elements are first loaded in memory and then returned as the iterator. |
| * | ||
| * Finds all keys from given iterable. | ||
| * Result is returned in an iterable of key-value pairs. | ||
| * If key is not found, null value is included in result pair. |
There was a problem hiding this comment.
Suggested change
| * If key is not found, null value is included in result pair. | |
| * If key is not found, None value is included in result pair. |
| /** | ||
| * Registry of opened LevelDB instances. | ||
| * LevelDB prohibit access to the same storage file from more than one DB instance. | ||
| * And ergo application (mostly tests) quite frequently doesn't not explicitly close |
There was a problem hiding this comment.
Suggested change
| * And ergo application (mostly tests) quite frequently doesn't not explicitly close | |
| * And ergo application (mostly tests) quite frequently doesn't close explicitly |
There was a problem hiding this comment.
explicitly close is a perfect word order
There was a problem hiding this comment.
I don't get the meaning of the sentence in this case.
| lock.writeLock().lock() | ||
| try { | ||
| add(path, factory.open(path, options)) | ||
| } catch { |
There was a problem hiding this comment.
Fix formatting and identation
.../src/test/scala/scorex/crypto/authds/avltree/batch/VersionedLDBAVLStorageSpecification.scala
Show resolved
Hide resolved
avldb/src/test/scala/scorex/crypto/authds/avltree/batch/benchmark/IODBBenchmark.scala
Show resolved
Hide resolved
|
@aslesarenko thanks for the comments, I've addressed all of them! |
aslesarenko
approved these changes
Dec 26, 2019
Open
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.
This PR builds on top #969 . In addition the PR #969 , this one is using LDBVersionedStore for wallet registry as well. Old class VersionedLDBKVStore currently is used in benchmarks only , and to be removed.
Some ByteArrayWrapper usages eliminated as well, and IODB with ByteArrayWrapper are to be removed in next versions as well.