-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[Fix]
Part-2 - LevelDbStore
#3415
base: master
Are you sure you want to change the base?
[Fix]
Part-2 - LevelDbStore
#3415
Conversation
// Keep whole blockchain open plus future files | ||
// at lease up to block index 10_000_000 | ||
MaxOpenFiles = 4096, | ||
FilterPolicy = 10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FilterPolicy = 10, | |
FilterPolicy = Native.leveldb_filterpolicy_create_bloom(15), |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for extra, 5 bits. Bloom filters should be as small as possible for datasets without that many collisions. 10 is way more than enough. Speed is key...
? _db.Seek(prefix, new ReadOptions { FillCache = false, }) | ||
: _db.SeekPrev(prefix, new ReadOptions { FillCache = false, }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? _db.Seek(prefix, new ReadOptions { FillCache = false, }) | |
: _db.SeekPrev(prefix, new ReadOptions { FillCache = false, }); | |
? _db.Seek(prefix, ReadOptions.Default) | |
: _db.SeekPrev(prefix, ReadOptions.Default); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cause problem, because ReadOptions.Default
is static readonly
and will use the same instance. So if someone changes ReadOptions.Default
parameters will cause problems for us. Thats why I coded it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReadOptions.Default
isnt the same as new ReadOptions { FillCache = false, }
if (disposing) | ||
{ | ||
FreeManagedObjects(); | ||
} | ||
if (Handle != nint.Zero) | ||
{ | ||
FreeUnManagedObjects(); | ||
Handle = nint.Zero; | ||
} | ||
_disposed = true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (disposing) | |
{ | |
FreeManagedObjects(); | |
} | |
if (Handle != nint.Zero) | |
{ | |
FreeUnManagedObjects(); | |
Handle = nint.Zero; | |
} | |
_disposed = true; | |
} | |
_disposed = true; | |
if (disposing) | |
{ | |
FreeManagedObjects(); | |
} | |
if (Handle != nint.Zero) | |
{ | |
FreeUnManagedObjects(); | |
Handle = nint.Zero; | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leveldb handle
isnt disposed
until FreeManagedObjects()
is completed and Handle
is zeroed. Even though dotnet
class
is disposed
you can still call dispose
method of a disposed
class
to dispose
again.
Description
This fixes the store's
dotnet
memory management forGC
, a long with adding new feature options, easierException
catching.Change Log
LevelDbStoreTests
for unit testingDB
andIterator
Type of change
How Has This Been Tested?
Unit tests
Checklist: