Skip to content
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

Changed LiteDB to v4 for stability. #242

Merged
merged 16 commits into from
Jan 8, 2021
Merged

Changed LiteDB to v4 for stability. #242

merged 16 commits into from
Jan 8, 2021

Conversation

DennisAMenace
Copy link
Contributor

Discussion in Discord, Dan will add ReadyData for the tests before this is added in.


var sql = "SELECT " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How confident are you about this?
I am not sure we have tests here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Give me a day to run through some manual testing, I will let you know when it is ready.

Copy link
Contributor Author

@DennisAMenace DennisAMenace Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dangershony, this is ready as I've made the final adjustments. I've done all of my manual tests, but I cannot run the automated unit tests because the ReadyData needs to be regenerated.

How confident are you about this?
I am not sure we have tests here.

There are a couple tests for this method, but we need the readydata to be regenerated for it.

@dangershony
Copy link
Member

I will pull the branch to merge master and regenerate the tests

@DennisAMenace
Copy link
Contributor Author

I merged this with master to get the fixed tests. Any help to finish this PR would be good @dangershony as it will fix our stability issues - thank you.

The file wallet-with-funds.db needs to be regenerated with the right amount of blocks, or change this to be automatic.
image
https://github.com/block-core/blockcore/blob/master/src/Tests/Blockcore.IntegrationTests/Wallet/Data/wallet-with-funds.db

@dangershony
Copy link
Member

I am not sure what was decided with this problem? do we downgrade? @sondreb

- Rename the database file name for addressindexer. This means users must manually delete the old file to clear up disk space.
- Attempt to read the wallet database and if it fails, attempt to rename it away and try again (only one time).
- The "affectedAddresses" query fails and must be updated to work properly with LiteDB V4.
- This fixes a crash that occured due to LINQ resulting in an invalid query against V4 of LiteDB. Reverted back to old syntax.
- Improves the purge performance greatly by using query to perform a bulk delete.
@sondreb
Copy link
Member

sondreb commented Jan 8, 2021

I have improved the V4 performance and we'll merge this. An update to wallet test files is needed to ensure that integration tests works.

@sondreb sondreb merged commit d4dcd41 into block-core:master Jan 8, 2021
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.

3 participants