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
Problem: Events API is unreliable #2529
Conversation
@vrde I put two issues that this PR "Fixes" in the initial comment. Can you verify that I put the correct issues there? |
yield {'height': block['height'], | ||
'asset_id': asset_id, | ||
'transaction_id': tx['id']} | ||
'transaction_id': tx.id} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're busy fixing the events API, we may as well fix the docs about it. If you look in the docs subsection
http://docs.bigchaindb.com/projects/server/en/master/events/websocket-event-stream-api.html#id3
it gives this "Example message:"
{
"transaction_id": "<sha3-256 hash>",
"asset_id": "<sha3-256 hash>",
"block_id": "<int>"
}
Clearly, "block_id"
should be replaced by "height"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence right before that also refers to the "containing block’s ID." but it should say "containing block's height."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"block_id": "<int>"
is an int
wrapped in a string
. Should we just use int
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll use int
. Given that the biggest integer in JavaScript is 9007199254740991
, and if a BigchainDB network creates a block every second, we will have problems in 286 million years.
At that point in time we would have other problems to deal with, since all the continents on Earth may fuse into a supercontinent (from Wikipedia).
d7e179b
to
70c3fb4
Compare
Codecov Report
@@ Coverage Diff @@
## master #2529 +/- ##
==========================================
+ Coverage 93.2% 94.02% +0.82%
==========================================
Files 44 43 -1
Lines 2547 2495 -52
==========================================
- Hits 2374 2346 -28
+ Misses 173 149 -24 |
6b29818
to
51fed0d
Compare
Solution: Stop getting events from the Tendermint events API. Get the event right before returning from the COMMIT phase of Tendermint, and publish it to the events queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good. It would be good to test how reliable is the events api now under stress
I'm implicitly testing this with the performance tests. I'm able to push 1M transactions and get them through the WebSocket API without any problem. Before it was not possible. I also think this way of getting events removes some load from both BigchainDB and Tendermint (with the previous version of the code, I saw some delay between committing a block and having the transactions through the WebSocket). |
This PR also closes #2414. |
Solution: Stop getting events from the Tendermint events API. Get the
event right before returning from the COMMIT phase of Tendermint and
publish it to the events queue.
Fixes #2442
Fixes #2506
Fixes #2434