-
Notifications
You must be signed in to change notification settings - Fork 168
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
[Transaction Service Status] Batch status and memo writes to DB. #3026
base: master
Are you sure you want to change the base?
Conversation
This data was collected using a synthetic workload, from an internal test, and a 2-node local cluster setup running on a single physical node (Linux). This test runs in about a minute. The transaction mix is largely test transactions that have 35 account pubkeys including the fee payer [these land in TSS in 64 tx batches] and single vote transactions [they land in a batch of 1]. This was measured execution time for the match segment that handles transaction status batches in transaction status service.
I also ran the same synthetic workload and internal test on a small multi-node distributed private test cluster, and compared high level disk io metrics for num sectors written and num writes completed and the ratio were pretty close, and seemed to me the writes must generally be 128k commands based on the ratios averaged out. |
This is interesting given the high level timings seem significantly better after this change. Do we think this is just due to reduction in CPU time from merging writes internally? |
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.
Left a couple of suggestions.
I'm also wondering if we have an existing bencher for some of these operations. If not, might be nice to add one and confirm it shows benefits relative to the unbatched behavior.
ledger/src/blockstore.rs
Outdated
@@ -9618,6 +9653,7 @@ pub mod tests { | |||
.map(|key| (key, true)), | |||
TransactionStatusMeta::default(), | |||
counter, | |||
None, | |||
) |
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.
We should probably have a unit test that confirms the new batching behavior
@lijunwangs - I'm thinking you're the best person to review these changes while Tyera is out. Let me know if there is a better candidate |
What is the exact metrics being used for this data? |
@lijunwangs It's not an official metric that uploads to the metrics db, but I posted the debug code used for timing it here. |
688214f
to
905cc0e
Compare
2a45713
to
10b05e0
Compare
Problem
Transaction status service issues individual writes to DB backend for each transaction memo, and each account/pubkey update within a transaction. This seems inefficient at least from CPU execution time perspective, even if the DB backend is aggregating and batching IO updates to disk.
Summary of Changes