SQLite migration code #10551

kiminuo · 2023-04-24T10:31:58Z

Follow-up to #10272
The important commit is 85f1e4a.

Takes MatureIndex.dat and converts it to IndexStore.sqlite. ~~Unfortunately, the process is not very fast. It takes 43 seconds on my machine for testnet. So this PR would benefit from giving a user a visual notification that the migration is in progress. WDYT?~~ We agreed that we should not add any UX.

Note: This PR does not remove the original MatureIndex.dat as agreed in a previous meeting. That is a next logical step to avoid issues for developers.

Testing

Remove C:\Users\<USER>\AppData\Roaming\WalletWasabi\Client\BitcoinStore\<NETWORK>\IndexStore\IndexStore.sqlite and IndexStore.sqlite should be created during app's launch.

Measurements

On my machine:

For main network: It takes about 21 seconds.
For testnet network: It takes about 8 seconds.

WalletWasabi/Stores/BlockFilterSqliteStorage.cs

…ith-sqlite' into feature/2023-04-24-Sqlite-migration # Conflicts: # WalletWasabi/Stores/IndexStore.cs

…ith-sqlite' into feature/2023-04-24-Sqlite-migration

molnard · 2023-04-25T11:57:11Z

Sorry @kiminuo I misunderstood something in yesterday's meetings. I thought it takes 10 minutes to migrate but I see it is less than 1 minute. That is acceptable UX. Where will the user wait for the migration?

molnard · 2023-04-25T13:11:07Z

giving a user a visual notification that the migration is in progress. WDYT?

Do not go down that road, it is long and complicated and not worth it for just only 1 minute (or less). It is enough if wallet loading will take longer.

MaxHillebrand

tACK 2395e63

On the first run:

INFO	IndexStore.MigrateToSqliteNoLock (124)	Filters to add is 1601354. Elapsed seconds 6.
INFO	IndexStore.MigrateToSqliteNoLock (127)	Migration to SQLite was finished in 13 seconds.
INFO	IndexStore.Dispose (77)	InitializeAsync finished in 13.12 seconds.

…Sqlite-migration

kiminuo · 2023-04-25T21:28:13Z

Where will the user wait for the migration?

It happens on the initial page which is frozen at the moment. You can actually try it. I feel like the UX is not great at the moment but maybe it's acceptable given it's one time thing. Maybe we should just mention it in the next release notes so that people are not suprised.

Using https://www.meziantou.net/split-a-string-into-lines-without-allocation.htm would shorten the time required for the operation by about half, I suspect, because we allocate many and many strings by reading the MatureIndex.dat file. But I have not tried it because it starts to feel a bit over the top.

At the moment, I don't have another good idea how to make it faster without non-trivial time investment. Maybe someone else has an idea how to make it faster?

kiminuo · 2023-04-25T21:28:57Z

@MaxHillebrand It should be slightly faster now.

MaxHillebrand

TACK c586c42

30 seconds now.

I see the log before the UI opens, so loading time is seriously not an issue.

kiminuo · 2023-04-26T07:34:06Z

WalletWasabi/Stores/BitcoinStore.cs

@@ -44,6 +44,9 @@ public class BitcoinStore

 	public async Task InitializeAsync(CancellationToken cancel = default)
 	{
+		// Switch to a different thread so that we can confinue with other initialization tasks and UI can show up.
+		await Task.Yield();


Not entirely necessary but without it, I can see white screen instead of the initial Avalonia page.

kiminuo · 2023-04-26T07:35:28Z

WalletWasabi/Backend/Models/FilterModel.cs

 	{
 		Header = header;
-		_filter = filter;
+		FilterData = filterData;
+		_filter = new(() => new GolombRiceFilter(filterData, 20, 1 << 20), LazyThreadSafetyMode.ExecutionAndPublication);


Accessing filter bytes through GolombRiceFilter is slow. It's better to touch GolombRiceFilter only when really necessary.

kiminuo · 2023-04-26T07:36:13Z

WalletWasabi/Stores/IndexStore.cs

+					if (i % 100_000 == 0)
+					{
+						IndexStorage.BulkAppend(filters);
+						filters.Clear();
+					}


It seems it's slightly faster when "chunked". Not by much.

WalletWasabi/Stores/IndexStore.cs

molnard · 2023-04-26T10:42:39Z

The period of the migration is OK. Regarding the UX - can this migration take place exactly the same way as it was before with processing the filters? There was no UI freeze when the filters were not ready.

adamPetho · 2023-04-26T15:37:12Z

TestNet ⚡

2023-04-26 17:29:40.380 [1] WARNING     IndexStore.MigrateToSqliteNoLock (99)   Migration of block filters to SQLite format is about to begin. Please wait a moment.
2023-04-26 17:29:45.121 [1] INFO        IndexStore.MigrateToSqliteNoLock (137)  Migration of 1602357 filters to SQLite was finished in 00:00:04.7387223 seconds.

MainNet

2023-04-26 17:50:16.475 [1] WARNING     IndexStore.MigrateToSqliteNoLock (99)   Migration of block filters to SQLite format is about to begin. Please wait a moment.
2023-04-26 17:50:31.992 [1] INFO        IndexStore.MigrateToSqliteNoLock (137)  Migration of 305180 filters to SQLite was finished in 00:00:15.5153843 seconds.

My only nitpick is that the UI is totally frozen while the migration is happening.

molnard · 2023-04-26T15:42:49Z

Make sure when testing this, that wallet from the latest release will be loaded as fast as with this PR. There should not be a noticeable change.

…Sqlite-migration

kiminuo · 2023-04-26T20:58:53Z

I have attempted to modify the implementation to avoid the frozen UI issue. However, now it's a bit more involved and a bit more risky. So it requires more careful review.

It's also a bit hackish. The idea is that when we get first filters batch, then we just run migration and fix SmartHeaderChain. It's not optimal. It's not very nice. However, it does not freeze the UI.

If anyone has a better idea how to approach this, I'm all ears.

WalletWasabi/Stores/IndexStore.cs

molnard

NACK for the current solution as you said is a hack - from the worst kind. I am sure you can find it better, please elaborate more.

WalletWasabi/Stores/IndexStore.cs

kiminuo · 2023-04-29T07:55:46Z

NACK for the current solution as you said is a hack - from the worst kind. I am sure you can find it better, please elaborate more.

I'm not sure what constitutes the worst kind of hack. What are the properties you don't like?

Thinking about it a bit more, I believe we can put the code here

WalletWasabi/WalletWasabi/Services/WasabiSynchronizer.cs

Line 123 in e7f4667

Logger.LogTrace("> Wasabi synchronizer thread starts.");

. That seems like that might behave slightly better (run the migration a bit sooner). But it will still be hacky. The thing is that one has to argue real well that the migration won't break anything and that's not entirely easy given that SmartHeaderChain is used on many and many places. So I believe the original approach was actually better (maybe try to make it even faster so that it does not mind too much that GUI is not responsive?).

molnard · 2023-05-01T13:48:43Z

I'm not sure what constitutes the worst kind of hack

The AddNewFiltersAsync method can start the migration in case.

molnard · 2023-05-01T13:55:41Z

I am not sure I understand. I put this 10 sec delay here and I did not get a UI freeze.

…Sqlite-migration # Conflicts: # WalletWasabi/Stores/IndexStore.cs

This reverts commit d1fa430. # Conflicts: # WalletWasabi/Stores/IndexStore.cs

kiminuo · 2023-05-01T17:37:42Z

I am not sure I understand. I put this 10 sec delay here and I did not get a UI freeze.

I reverted to my original approach and added your X second delay trick (I'm not entirely sure why it works though).

MaxHillebrand · 2023-05-02T11:27:17Z

What's needed to push this over the finish line?
(other than me cheering you on)

kiminuo · 2023-05-02T11:40:15Z

David's ACK/NACK on the approach. Review&testing by others will help too.

molnard

tested it works perfectly. One concern to address.

molnard · 2023-05-02T14:13:14Z

WalletWasabi/Stores/IndexStore.cs

+		int i = 0;
+
+		// Helps to avoid UI freezing.
+		await Task.Delay(5_000, cancellationToken).ConfigureAwait(false);


This help only because it is giving back the execution to the caller thread. When it returns here it might be running on a separate thread after (.ConfigureAwait(false)) , might not - it is not guaranteed. If ThreadPoll will assign the task to the UI thread (random) it will still freeze the UI.

Because you are not having an async method for MigrateToSqliteNoLockAsync (perf reasons cACK) I suggest removing the Delay and changing it back to the sync method.

Then call it with

await Task.Run(MigrateToSqliteNoLockAsync);

There is already using (await IndexLock.LockAsync(cancellationToken).ConfigureAwait(false)), so I added:

await Task.Run(MigrateToSqliteNoLock, cancellationToken).ConfigureAwait(false);

Or is there any reason why it should be done differently?

MarnixCroes

FWIW (maybe edge case, maybe useful):
My client was busy doing full rescan/filter download.
I stopped the process halfway, then checked out and ran this PR.
The log immediately says it downloaded the latest block filter (so no more filter download), however, the wallet load screen is still counting down where it left off: many hours remaining. While in logs nothing is changing.

turbolay · 2023-05-02T16:35:09Z

FWIW (maybe edge case, maybe useful):
My client was busy doing full rescan/filter download.
I stopped the process halfway, then checked out and ran this PR.
The log immediately says it downloaded the latest block filter (so no more filter download), however, the wallet load screen is still counting down where it left off: many hours remaining. While in logs nothing is changing.

This is expected, see second part of #10589 (comment)

molnard

tACK. 🥇

molnard · 2023-05-03T10:38:09Z

The next step is to delete the old files. Two weeks from now that PR can be merged. 17th of May. Added to the calendar.

Implement migration

85f1e4a

pull-request-size bot added the size/XXL label Apr 24, 2023

molnard mentioned this pull request Apr 24, 2023

Introduce sqlite for storing of filters for IndexStore. #10272

Merged

kristapsk reviewed Apr 24, 2023

View reviewed changes

WalletWasabi/Stores/BlockFilterSqliteStorage.cs Outdated Show resolved Hide resolved

WalletWasabi/Stores/BlockFilterSqliteStorage.cs Outdated Show resolved Hide resolved

WalletWasabi/Stores/BlockFilterSqliteStorage.cs Outdated Show resolved Hide resolved

kiminuo added 4 commits April 25, 2023 09:30

Merge remote-tracking branch 'kiminuo/feature/2023-03-05-IndexStore-w…

15a2bf7

…ith-sqlite' into feature/2023-04-24-Sqlite-migration # Conflicts: # WalletWasabi/Stores/IndexStore.cs

Merge remote-tracking branch 'kiminuo/feature/2023-03-05-IndexStore-w…

99aa57e

…ith-sqlite' into feature/2023-04-24-Sqlite-migration

Some progress

f08bdba

Faster

2395e63

MaxHillebrand previously approved these changes Apr 25, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into feature/2023-04-24-…

3302f5a

…Sqlite-migration

pull-request-size bot added size/L and removed size/XXL labels Apr 25, 2023

Faster approach

71788bd

kiminuo dismissed MaxHillebrand’s stale review via 71788bd April 25, 2023 20:43

Bit faster

9cd93b3

Make DeepSource happy

c586c42

MaxHillebrand previously approved these changes Apr 26, 2023

View reviewed changes

kiminuo marked this pull request as ready for review April 26, 2023 07:33

kiminuo commented Apr 26, 2023

View reviewed changes

WalletWasabi/Stores/IndexStore.cs Show resolved Hide resolved

Remove TODO from the previous PR

e1556a1

kiminuo added 2 commits April 26, 2023 22:49

Refactor migration code

d1fa430

Merge remote-tracking branch 'origin/master' into feature/2023-04-24-…

cbc0625

…Sqlite-migration

kiminuo dismissed MaxHillebrand’s stale review via cbc0625 April 26, 2023 20:51

CF

2e583bd

molnard requested changes Apr 28, 2023

View reviewed changes

WalletWasabi/Stores/IndexStore.cs Outdated Show resolved Hide resolved

molnard requested changes Apr 28, 2023

View reviewed changes

WalletWasabi/Stores/IndexStore.cs Show resolved Hide resolved

WalletWasabi/Stores/IndexStore.cs Show resolved Hide resolved

MaxHillebrand mentioned this pull request Apr 30, 2023

Something The past few days Triggers Rescan of Wallets #10589

Closed

kiminuo added 3 commits May 1, 2023 19:27

Merge remote-tracking branch 'origin/master' into feature/2023-04-24-…

4142eb4

…Sqlite-migration # Conflicts: # WalletWasabi/Stores/IndexStore.cs

Revert "Refactor migration code"

9a8dea0

This reverts commit d1fa430. # Conflicts: # WalletWasabi/Stores/IndexStore.cs

David's approach

e719552

Better

065a4e7

molnard requested changes May 2, 2023

View reviewed changes

MarnixCroes reviewed May 2, 2023

View reviewed changes

suggestion

8d3515c

molnard approved these changes May 3, 2023

View reviewed changes

molnard merged commit 9f2b1c4 into zkSNACKs:master May 3, 2023
6 of 7 checks passed

kiminuo deleted the feature/2023-04-24-Sqlite-migration branch May 3, 2023 12:23

kiminuo mentioned this pull request May 18, 2023

[Index][Migration] Remove old index data #10731

Merged

molnard mentioned this pull request May 18, 2023

[Test] Deployment procedure report #10735

Closed

molnard mentioned this pull request Jun 27, 2023

Test vectors for v2.0.4 #10945

Closed

kiminuo mentioned this pull request Aug 14, 2023

Make filter migration cancellable #11274

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQLite migration code #10551

SQLite migration code #10551

kiminuo commented Apr 24, 2023 •

edited by nopara73

molnard commented Apr 25, 2023

molnard commented Apr 25, 2023

MaxHillebrand left a comment

kiminuo commented Apr 25, 2023 •

edited

kiminuo commented Apr 25, 2023

MaxHillebrand left a comment

kiminuo Apr 26, 2023

kiminuo Apr 26, 2023

kiminuo Apr 26, 2023

molnard commented Apr 26, 2023

adamPetho commented Apr 26, 2023 •

edited

molnard commented Apr 26, 2023

kiminuo commented Apr 26, 2023 •

edited

molnard left a comment

kiminuo commented Apr 29, 2023 •

edited

molnard commented May 1, 2023

molnard commented May 1, 2023

kiminuo commented May 1, 2023

MaxHillebrand commented May 2, 2023

kiminuo commented May 2, 2023 •

edited

molnard left a comment

molnard May 2, 2023 •

edited

kiminuo May 2, 2023

MarnixCroes left a comment

turbolay commented May 2, 2023 •

edited

molnard left a comment

molnard commented May 3, 2023

SQLite migration code #10551

SQLite migration code #10551

Conversation

kiminuo commented Apr 24, 2023 • edited by nopara73

Testing

Measurements

molnard commented Apr 25, 2023

molnard commented Apr 25, 2023

MaxHillebrand left a comment

Choose a reason for hiding this comment

kiminuo commented Apr 25, 2023 • edited

kiminuo commented Apr 25, 2023

MaxHillebrand left a comment

Choose a reason for hiding this comment

kiminuo Apr 26, 2023

Choose a reason for hiding this comment

kiminuo Apr 26, 2023

Choose a reason for hiding this comment

kiminuo Apr 26, 2023

Choose a reason for hiding this comment

molnard commented Apr 26, 2023

adamPetho commented Apr 26, 2023 • edited

molnard commented Apr 26, 2023

kiminuo commented Apr 26, 2023 • edited

molnard left a comment

Choose a reason for hiding this comment

kiminuo commented Apr 29, 2023 • edited

molnard commented May 1, 2023

molnard commented May 1, 2023

kiminuo commented May 1, 2023

MaxHillebrand commented May 2, 2023

kiminuo commented May 2, 2023 • edited

molnard left a comment

Choose a reason for hiding this comment

molnard May 2, 2023 • edited

Choose a reason for hiding this comment

kiminuo May 2, 2023

Choose a reason for hiding this comment

MarnixCroes left a comment

Choose a reason for hiding this comment

turbolay commented May 2, 2023 • edited

molnard left a comment

Choose a reason for hiding this comment

molnard commented May 3, 2023

kiminuo commented Apr 24, 2023 •

edited by nopara73

kiminuo commented Apr 25, 2023 •

edited

adamPetho commented Apr 26, 2023 •

edited

kiminuo commented Apr 26, 2023 •

edited

kiminuo commented Apr 29, 2023 •

edited

kiminuo commented May 2, 2023 •

edited

molnard May 2, 2023 •

edited

turbolay commented May 2, 2023 •

edited