-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Pooling Wallet - one possible root cause for syncing issues #12853
Comments
Thank you for your analysis. Before we continue the discussion, I would like to get on the same page on what is the use case you are describing. We did not optimize the pool wallet for 70k state transitions. We have optimized for many standard transactions, not pool wallet singletons. Do you have a farmer that has won 70k blocks? Or have you changed your pool on chain 70k times? Why exactly do you have so many transactions for the pool wallet? |
The data is from the keys we used to test our pool on testnet, farming for a very long time and with a decent amount of testnet plots. So once joined the pool all other events are from block wins. From a backup db we know, that the last known number of pool state transitions is 28370. I actually had time for a few days to review the wallet syncing from a DB perspective, found also a few minor things, |
I see, so it's testnet keys. If there are some relatively simple ways to optimize it, like not re-reading the whole table, we can make these changes. |
I had the feeling, that this is an extreme edge case, but I still think this could be avoided. This are the SQL statement executed for every loop: 15:44:45.401262 SELECT height, coin_spend FROM pool_state_transitions WHERE wallet_id=? ORDER BY transition_index
15:44:45.410667 SELECT transition_index, height, coin_spend FROM pool_state_transitions WHERE wallet_id=? ORDER BY transition_index DESC LIMIT 1
15:44:45.412169 SELECT COUNT(*) FROM pool_state_transitions WHERE wallet_id=? AND height=? AND coin_spend=?
15:44:45.412477 INSERT OR IGNORE INTO pool_state_transitions VALUES (?, ?, ?, ?)
15:44:45.412934 SELECT height, coin_spend FROM pool_state_transitions WHERE wallet_id=? ORDER BY transition_index
15:44:45.422234 SELECT height, coin_spend FROM pool_state_transitions WHERE wallet_id=? ORDER BY transition_index
15:44:45.453788 SELECT height, coin_spend FROM pool_state_transitions WHERE wallet_id=? ORDER BY transition_index
15:44:45.549240 SELECT * from coin_record WHERE coin_name=?
15:44:45.549638 SELECT * from coin_record WHERE coin_name=?
15:44:45.549939 INSERT OR FAIL INTO coin_record VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
15:44:45.550090 SELECT last_insert_rowid()
15:44:45.550250 SELECT MIN(derivation_index) FROM derivation_paths WHERE used=0 AND hardened=0;
15:44:45.550484 SELECT MAX(derivation_index) FROM derivation_paths WHERE wallet_id=?
15:44:45.550930 UPDATE coin_record SET spent_height=?,spent=? WHERE coin_name=?
15:44:45.551016 SELECT last_insert_rowid() The pool_state_transition table is even 4 times selected, not 3 times. I missed the And another thing, the comment on chia-blockchain/chia/pools/pool_wallet.py Lines 263 to 265 in 7c9fb65
But the whole apply process runs in a single transaction, which also blows up the wal file and of course, when beeing stopped or crashed it has to start from 0 again. |
This applies to pool farming, as well as self farming with plot NFTs. Yes it is extremely inefficient. Could you provide timings on each of those queries and which ones took a long time? We might be able to fix it by adding a simple cache somewhere. However, I do not want to make extensive and potentially risky changes, since it's not an issue that will affect too many people, and adding a bug would affect many people |
From what I see, the inefficiency comes mainly (or only) from querying the growing Short reportTIME: {amount of seconds to loop 200 times}, NEW_COIN_ITERATIONS: { overall counter } The following metrics show the avg and sum amount of time spent, measured before and after this for loop: chia-blockchain/chia/pools/pool_wallet.py Line 289 in 7c9fb65
AVG_OVER_LAST_200_ITERS: {average in sec for the last 200 times}
For instance, the first 200 iterations:
You could read as, it took about 15.3s overall. Between iteration 3600 and 3800:
The processing of these 200 iterations took 77.7s. |
I have a test wallet on testnet with many claims and wins, and it is days to sync - I believe it is likely partially explained by this analysis. Assigning to Almog to take a look |
The
long_sync
operation of a big wallet, when the wallet has manypool_state_transitions
, is very inefficient.I've tested sync operations with a testnet wallet with ~80k coins and assumable ~70k pool_state_transitions.
Process Flow
Here is a simplified process flow of how I understood the relevant parts of the sync.
When the
long sync
begins with the Pooling Wallet sync, this loop is executed:chia-blockchain/chia/wallet/wallet_state_manager.py
Line 1087 in 244a9dc
The inefficiency comes from the call of
apply_state_transition
function.chia-blockchain/chia/wallet/wallet_state_manager.py
Line 1091 in 244a9dc
Within this function we first call another function to add the spend to the db:
chia-blockchain/chia/pools/pool_wallet.py
Line 284 in 244a9dc
BEGIN
add_spend
Before the spend is actually added to the db, the following two queries are executed:
chia-blockchain/chia/wallet/wallet_pool_store.py
Lines 47 to 52 in 244a9dc
chia-blockchain/chia/wallet/wallet_pool_store.py
Lines 61 to 64 in 244a9dc
END
add_spend
Then we enter this loop
chia-blockchain/chia/pools/pool_wallet.py
Lines 288 to 297 in 244a9dc
Where we basically select all known spends from the pooling wallet:
chia-blockchain/chia/pools/pool_wallet.py
Line 290 in 244a9dc
To get all spends, this SQL query is executed:
chia-blockchain/chia/wallet/wallet_pool_store.py
Lines 97 to 98 in 244a9dc
Then we process the next coin, starting the whole loop again.
Possible Problems
For every coin, we select the whole
pool_state_transitions
table 3 times.To get the transition_index from the wallet, starting here:
chia-blockchain/chia/wallet/wallet_pool_store.py
Line 47 in 244a9dc
What is the reason for this query? If we only want to get the highest transition_index for the given_wallet, then we could probably optimize the query.
To get the amount of transitions for a given wallet, height and spend, starting here:
chia-blockchain/chia/wallet/wallet_pool_store.py
Line 61 in 244a9dc
I guess this query checks if the current spend should be added to the database.
The check is done by selecting rows filtered by
wallet_id
andheight
andcoin_spend
.The PK of the table
pool_state_transitions
istransition_index, wallet_id
and there are no other indexes.To get all heights and coin_spends from a given wallet, starting here:
chia-blockchain/chia/wallet/wallet_pool_store.py
Line 97 in 244a9dc
So, every time we loop through the function
apply_state_transition
, we most probably add a new spend and select all old spends plus the new spends three times (2 times the whole table, once the rowcount), basically reading the whole table, like explained above.This adds up real quick and leads to a lot of redundant, unnecessary(?) work.
I don't really understand the design decissions, why the sync, in regards of the SQLite DB part, is implemented that way,
but I'm pretty sure this very problem could be mitigated by, at least not reading the growing whole table again and again.
The text was updated successfully, but these errors were encountered: