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

execute SQL updates as chunks in _set_spent function for tx_removals #11613

Merged
merged 4 commits into from
Jul 13, 2022
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions chia/full_node/coin_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,22 +518,28 @@ async def _set_spent(self, coin_names: List[bytes32], index: uint32):
if len(coin_names) == 0:
return

updates = []
for coin_name in coin_names:
updates.append((index, self.maybe_to_hex(coin_name)))

async with self.db_wrapper.write_db() as conn:
if self.db_wrapper.db_version == 2:
ret: Cursor = await conn.executemany(
"UPDATE OR FAIL coin_record SET spent_index=? WHERE coin_name=? AND spent_index=0", updates
)

else:
ret = await conn.executemany(
"UPDATE OR FAIL coin_record SET spent=1,spent_index=? WHERE coin_name=? AND spent_index=0",
updates,
)
if ret.rowcount != len(coin_names):
rows_updated: int = 0
for coin_names_chunk in chunks(coin_names, SQLITE_MAX_VARIABLE_NUMBER):
name_params = ",".join(["?"] * len(coin_names_chunk))
if self.db_wrapper.db_version == 2:
ret: Cursor = await conn.execute(
f"UPDATE OR FAIL coin_record INDEXED BY sqlite_autoindex_coin_record_1 "
Copy link
Contributor

Choose a reason for hiding this comment

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

specifying the index here, I assume, is to make sure it's not using the spent_index index, right?
The condition spent_index=0 isn't really supposed to be required. All these coins are supposed to be unspent (iiuc). Perhaps we could remove that condition and also drop the INDEXED BY, would that work or make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I ran this first without that index clause and it started to create a huge amount of i/o, then I figured the wrong index usage.
I'm sure if we remove spent_index condition, we don't need INDEXED BY any more

f"SET spent_index={index} "
f"WHERE spent_index=0 "
f"AND coin_name IN ({name_params})",
coin_names_chunk,
)
else:
ret = await conn.execute(
f"UPDATE OR FAIL coin_record INDEXED BY sqlite_autoindex_coin_record_1 "
f"SET spent=1, spent_index={index} "
f"WHERE spent_index=0 "
f"AND coin_name IN ({name_params})",
[name.hex() for name in coin_names_chunk],
)
rows_updated += ret.rowcount
if rows_updated != len(coin_names):
raise ValueError(
f"Invalid operation to set spent, total updates {ret.rowcount} expected {len(coin_names)}"
f"Invalid operation to set spent, total updates {rows_updated} expected {len(coin_names)}"
)