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

refactor: Optimize input and output storage #2672

Merged
merged 1 commit into from Jun 25, 2023

Conversation

yanguoyu
Copy link
Collaborator

@yanguoyu yanguoyu commented May 18, 2023

To search others' addresses at the history page, I create a table tx_lock to save tx_hash map lockHash

  1. Only save local wallets' lock into input and output, and save others' cell lockHash to tx_lock
  2. When searching by address, search addresses from input, output and tx_lock.
  3. When getting transaction detail, get the transaction detail from RPC.
  4. Record the last block number when caching tx to indexer_tx_hash_cache

@Keith-CY
Copy link
Collaborator

I'm not sure if I get the point.

Seems that all inputs and outputs are saved into inputs, outputs, others_input, and others_output.

It speeds searching up, but the storage is not optimized as mentioned at Magickbase/neuron-public-issues#77 (comment)

If a transaction includes many cells, they are still all saved into the database. The disk usage is not ameliorated.

@yanguoyu
Copy link
Collaborator Author

I'm not sure if I get the point.

Seems that all inputs and outputs are saved into inputs, outputs, others_input, and others_output.

It speeds searching up, but the storage is not optimized as mentioned at Magickbase/neuron-public-issues#77 (comment)

If a transaction includes many cells, they are still all saved into the database. The disk usage is not ameliorated.

Currently yes, I have not paid attention to the SQLite file's size, but only care about the search speed and SQL execution time. I want to use the data that has been got.

You remind me that we also pay attention to the SQLite file's size. Actually, I have tried another way to finish it, create a table that only saves tx hash and lockHash for searching addresses, and also works. Maybe it will reduce the disk cost by saving only two fields. I think saving two fields for searching addresses is cost-effective. But finally, for fewer RPC requests so I use currently schema.

For smaller SQLite'file size, there exist two schemas:

  1. Create a new table to save tx hash and lockHash for searching addresses.
  2. When searching others' addresses, get all transactions' detail in the current wallet and then filter with the address. I'm worry about the rpc's result is too big.

So, for reduce SQLite's file size, I suggestion for first solution. What do you think? @Keith-CY

@Keith-CY
Copy link
Collaborator

Keith-CY commented May 18, 2023

I'm not sure if I get the point.
Seems that all inputs and outputs are saved into inputs, outputs, others_input, and others_output.
It speeds searching up, but the storage is not optimized as mentioned at Magickbase/neuron-public-issues#77 (comment)
If a transaction includes many cells, they are still all saved into the database. The disk usage is not ameliorated.

Currently yes, I have not paid attention to the SQLite file's size, but only care about the search speed and SQL execution time. I want to use the data that has been got.

You remind me that we also pay attention to the SQLite file's size. Actually, I have tried another way to finish it, create a table that only saves tx hash and lockHash for searching addresses, and also works. Maybe it will reduce the disk cost by saving only two fields. I think saving two fields for searching addresses is cost-effective. But finally, for fewer RPC requests so I use currently schema.

For smaller SQLite'file size, there exist two schemas:

  1. Create a new table to save tx hash and lockHash for searching addresses.
  2. When searching others' addresses, get all transactions' detail in the current wallet and then filter with the address. I'm worry about the rpc's result is too big.

So, for reduce SQLite's file size, I suggestion for first solution. What do you think? @Keith-CY

Solution 2 has an alternative as mentioned at Magickbase/neuron-public-issues#77 (comment)

But I think solution 1 is good for our case, and it can be removed easily if a better solution is found

@yanguoyu
Copy link
Collaborator Author

Solution 2 has an alternative as mentioned at Magickbase/neuron-public-issues#77 (comment)

I'm worried about search one address that may belong to a miner.

@Keith-CY
Copy link
Collaborator

Solution 2 has an alternative as mentioned at Magickbase/neuron-public-issues#77 (comment)

I'm worried about search one address that may belong to a miner.

Indexer supports pagination so the count of transactions won't bother. But the pagination of indexer is cursor-style, while neuron uses offset-style it, it's a problem if we use indexer to list transactions.

So I think solution 1 is good for our case now

@yanguoyu
Copy link
Collaborator Author

I will reopen the PR after I changed it to Solution 1

@yanguoyu yanguoyu marked this pull request as draft May 18, 2023 07:51
@yanguoyu yanguoyu marked this pull request as ready for review May 21, 2023 14:25
@yanguoyu
Copy link
Collaborator Author

I will reopen the PR after I changed it to Solution 1

It's ready for review now.

@Keith-CY
Copy link
Collaborator

Keith-CY commented Jun 9, 2023

Please have a review @WhiteMinds @devchenyan @JeffreyMa597

@JeffreyMa597
Copy link
Contributor

please resolve the conflicts.

@github-actions
Copy link
Contributor

Packaging for test is done in 5249523509 for commit 3e9f263 .

@yanguoyu
Copy link
Collaborator Author

@FrederLu Please test it with the package.

@github-actions
Copy link
Contributor

Packaging for test is done in 5308946921 for commit cf25d37 .

@github-actions
Copy link
Contributor

Packaging for test is done in 5308951833 for commit 5e11fdf .

@Keith-CY
Copy link
Collaborator

Please resolve the conflicts and squash merge @yanguoyu

1. Storage others' cells tx and locks into tx-lock for search others' addresses.
2. Get transaction detail by rpc.
@github-actions
Copy link
Contributor

Packaging for test is done in 5369493403 for commit 8b41c58 .

@yanguoyu yanguoyu added this pull request to the merge queue Jun 25, 2023
Merged via the queue into nervosnetwork:develop with commit c89bd54 Jun 25, 2023
10 checks passed
@yanguoyu yanguoyu deleted the opt-local-db-save branch June 25, 2023 11:48
@Keith-CY Keith-CY mentioned this pull request Sep 20, 2023
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.

None yet

5 participants