rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block #26415

andrewtoth · 2022-10-28T15:27:04Z

For the getblock endpoint with verbosity=0, the rest_block REST endpoint for bin and hex, and zmq NotifyBlock we don't have to deserialize the block since we're just sending the raw data. This PR uses ReadRawBlockFromDisk instead of ReadBlockFromDisk to serve these requests, and only deserializes for verbosity > 0 and json REST requests. See benchmarks in #26684.

Benchmarked using ApacheBench. Requesting block 750,000 in binary 10k times on a single core (set -rest=1 in config):
ab -n 10000 -c 1 "http://127.0.0.1:8332/rest/block/0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e.bin"

On master, mean time 15ms.
On this branch, mean time 1ms.

For RPC

echo '{"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", 0]}' > /tmp/data.json
ab -p /tmp/data.json -n 1000 -c 1 -A user:password "http://127.0.0.1:8332/"

On master, mean time 32ms
On this branch, mean time 13ms

DrahtBot · 2022-10-28T21:42:10Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	achow101
Concept ACK	RandyMcMillan, kashifs, pablomartin4btc
Stale ACK	furszy, TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

andrewtoth · 2022-10-30T02:06:47Z

For some more context, here is the PR that introduced ReadRawBlockFromDisk #13151.

I see that this was attempted in #21319 as well.

andrewtoth · 2022-10-30T03:41:23Z

@luke-jr I wasn't aware of #21319 until now, but I can't comment there because it's locked. If you want to reopen that I can review and remove the change to RPC in this PR so it will only be for REST.

furszy · 2024-02-01T16:04:12Z

src/rpc/blockchain.cpp

As we only care about the data existence here, what about doing a more general check:

if (!(blockindex.nStatus & BLOCK_HAVE_DATA)) { throw JSONRPCError(RPC_MISC_ERROR, "Block not available"); }

(Unless you want to keep the same error message as before)

I think this check is specifically done to see if the block is pruned, hence the error message (pruned data). This is taken verbatim from the above function, GetBlockChecked, which all it does inside the locked block is check if it is pruned or not. Here we also extract the FlatFilePos. However, if blockindex.nStatus & BLOCK_HAVE_DATA is false then the call to ReadRawBlockFromDisk will fail and throw the message Block not available. The first commit addresses the undefined behavior part of calling ReadRawBlockFromDisk with a null FlatFilePos, which is safe to do now.

src/rpc/blockchain.cpp

src/rest.cpp

andrewtoth · 2024-02-21T23:42:00Z

@furszy @TheCharlatan updated with both your suggestions, thanks!

furszy

Code ACK 4fc91b38

sedited

Re-ACK 4fc91b38ae8c715e0ebf9abcb6da17044fe418b3

achow101 · 2024-03-12T16:20:24Z

ACK 4fc91b38ae8c715e0ebf9abcb6da17044fe418b3

achow101 · 2024-03-12T16:23:14Z

src/node/blockstorage.cpp

In 3f086a9353191a5df03aa595bdbb42c2195a23d2 "blockstorage: check nPos in ReadRawBlockFromDisk before seeking back"

Silent merge conflict here:

../../../src/node/blockstorage.cpp: In member function ‘bool node::BlockManager::ReadRawBlockFromDisk(std::vector<unsigned char>&, const FlatFilePos&) const’: ../../../src/node/blockstorage.cpp:1094:16: error: ‘error’ was not declared in this scope; did you mean ‘perror’? 1094 | return error("%s: OpenBlockFile failed for %s", __func__, pos.ToString()); | ^~~~~ | perror

error() was removed in #29236

@achow101 resolved, thanks.

ReadRawBlockFromDisk assumes a non-null pos that has an nPos >= 8. This simple check makes the function safer to call in the future, so callers don't need to worry about causing UB if the pos is null.

Note that for speed this commit also removes the proof of work and signet signature checks before returning the block in getblock. It is assumed if a block is stored it will be valid.

achow101 · 2024-03-12T17:01:54Z

re-ACK e710cef

Verified rebase diff.

ReadRawBlockFromDisk assumes a non-null pos that has an nPos >= 8. This simple check makes the function safer to call in the future, so callers don't need to worry about causing UB if the pos is null. Github-Pull: bitcoin#26415 Rebased-From: da338aa

Github-Pull: bitcoin#26415 Rebased-From: 38265cc

Github-Pull: bitcoin#26415 Rebased-From: 0865ab8

Note that for speed this commit also removes the proof of work and signet signature checks before returning the block in getblock. It is assumed if a block is stored it will be valid. Github-Pull: bitcoin#26415 Rebased-From: 95ce078

Note that for speed this commit also removes the proof of work and signet signature checks before returning the block in getblock. It is assumed if a block is stored it will be valid. Github-Pull: bitcoin#26415 Rebased-From: e710cef

andrewtoth force-pushed the read-raw-block branch from 1b62435 to e58dcb3 Compare October 28, 2022 15:40

andrewtoth marked this pull request as draft October 28, 2022 15:58

andrewtoth force-pushed the read-raw-block branch 2 times, most recently from 4986d7a to 01999de Compare October 28, 2022 18:05

andrewtoth marked this pull request as ready for review October 28, 2022 18:05

andrewtoth changed the title ~~rpc,rest: read raw block in getblock verbosity 0 and rest_block bin and hex~~ rpc,rest: faster getblock and rest_block by reading raw block Oct 28, 2022

andrewtoth force-pushed the read-raw-block branch from 01999de to d045ccf Compare October 30, 2022 03:40

andrewtoth marked this pull request as draft October 31, 2022 19:02

andrewtoth force-pushed the read-raw-block branch 5 times, most recently from 1ea00e8 to 3e5aebf Compare November 3, 2022 02:22

DrahtBot mentioned this pull request Nov 3, 2022

Add a "tx output spender" index #24539

Open

andrewtoth force-pushed the read-raw-block branch from 3e5aebf to e064571 Compare November 5, 2022 01:33

andrewtoth mentioned this pull request Nov 5, 2022

RPC/Blockchain: Optimise getblock for simple disk->hex case #21319

Closed

DrahtBot mentioned this pull request Nov 6, 2022

indexes: Stop using node internal types and locking cs_main, improve sync logic #24230

Draft

DrahtBot mentioned this pull request Nov 14, 2022

assumeutxo #15606

Closed

18 tasks

DrahtBot added the Needs rebase label Dec 8, 2022

andrewtoth force-pushed the read-raw-block branch from e064571 to e830c1f Compare December 11, 2022 02:10

andrewtoth marked this pull request as ready for review December 11, 2022 02:14

andrewtoth mentioned this pull request Dec 11, 2022

bench: add readblock benchmark #26684

Merged

andrewtoth force-pushed the read-raw-block branch from 129e7e8 to 3d7ae60 Compare January 23, 2024 01:08

DrahtBot removed the CI failed label Jan 23, 2024

furszy reviewed Feb 1, 2024

View reviewed changes

andrewtoth force-pushed the read-raw-block branch from 3d7ae60 to 4fc91b3 Compare February 21, 2024 22:38

furszy reviewed Mar 4, 2024

View reviewed changes

DrahtBot requested review from pablomartin4btc and sedited March 4, 2024 22:51

sedited approved these changes Mar 6, 2024

View reviewed changes

DrahtBot requested a review from achow101 March 12, 2024 16:20

achow101 reviewed Mar 12, 2024

View reviewed changes

andrewtoth force-pushed the read-raw-block branch from 4fc91b3 to 39bb816 Compare March 12, 2024 16:32

andrewtoth added 5 commits March 12, 2024 12:46

blockstorage: check nPos in ReadRawBlockFromDisk before seeking back

da338aa

ReadRawBlockFromDisk assumes a non-null pos that has an nPos >= 8. This simple check makes the function safer to call in the future, so callers don't need to worry about causing UB if the pos is null.

zmq: read raw block with ReadRawBlockFromDisk

38265cc

test: check more details on zmq raw block response

0865ab8

rpc: read raw block in getblock and deserialize for verbosity > 0

95ce078

Note that for speed this commit also removes the proof of work and signet signature checks before returning the block in getblock. It is assumed if a block is stored it will be valid.

rest: read raw block in rest_block and deserialize for json

e710cef

Note that for speed this commit also removes the proof of work and signet signature checks before returning the block in getblock. It is assumed if a block is stored it will be valid.

andrewtoth force-pushed the read-raw-block branch from 39bb816 to e710cef Compare March 12, 2024 16:48

DrahtBot requested review from furszy and sedited March 12, 2024 17:01

achow101 merged commit bde3db4 into bitcoin:master Mar 12, 2024

andrewtoth deleted the read-raw-block branch March 12, 2024 17:45

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Jun 13, 2024

zmq: read raw block with ReadRawBlockFromDisk

bbbcf92

Github-Pull: bitcoin#26415 Rebased-From: 38265cc

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Jun 13, 2024

test: check more details on zmq raw block response

b31a4a2

Github-Pull: bitcoin#26415 Rebased-From: 0865ab8

bitcoin locked and limited conversation to collaborators Mar 12, 2025

rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block #26415

rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block #26415

Uh oh!

Conversation

andrewtoth commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

andrewtoth commented Oct 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewtoth commented Oct 30, 2022

Uh oh!

furszy Feb 1, 2024

Choose a reason for hiding this comment

Uh oh!

andrewtoth Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewtoth commented Feb 21, 2024

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

achow101 commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achow101 Mar 12, 2024

Choose a reason for hiding this comment

Uh oh!

andrewtoth Mar 12, 2024

Choose a reason for hiding this comment

Uh oh!

achow101 commented Mar 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

andrewtoth commented Oct 28, 2022 •

edited

Loading

DrahtBot commented Oct 28, 2022 •

edited

Loading

andrewtoth commented Oct 30, 2022 •

edited

Loading

achow101 commented Mar 12, 2024 •

edited

Loading