API: Hotfix block endpoint #1397
Conversation
Eric-Warehime
requested review from
winder,
shiqizng,
AlgoStephenAkiki and
mciccarello
Codecov Report
@@ Coverage Diff @@
## hotfix/2.15.1 #1397 +/- ##
==============================================
Coverage 65.02% 65.02%
==============================================
Files 52 52
Lines 8311 8311
==============================================
Hits 5404 5404
Misses 2433 2433
Partials 474 474
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Co-authored-by: Will Winder <wwinder.unh@gmail.com>
idb/postgres/postgres.go
Outdated
| @@ -658,6 +658,9 @@ func buildTransactionQuery(tf idb.TransactionFilter) (query string, whereArgs [] | |||
| if tf.RekeyTo != nil && (*tf.RekeyTo) { | |||
| whereParts = append(whereParts, "(t.txn -> 'txn' -> 'rekey') IS NOT NULL") | |||
| } | |||
| if tf.ReturnRootTxnsOnly { | |||
| whereParts = append(whereParts, "t.txid IS NOT NULL") | |||
| } | |||
|
|
|||
| // If returnInnerTxnOnly flag is false, then return the root transaction | |||
| if !tf.ReturnInnerTxnOnly { | |||
There was a problem hiding this comment.
This is also needed to make sure we don't try to reference root:
| if !tf.ReturnInnerTxnOnly { | |
| if !tf.ReturnInnerTxnOnly && !tf.ReturnRootTxnsOnly { |
|
|
||
| // If returnInnerTxnOnly flag is false, then return the root transaction | ||
| if !tf.ReturnInnerTxnOnly { | ||
| if !(tf.ReturnInnerTxnOnly || tf.ReturnRootTxnsOnly) { |
There was a problem hiding this comment.
The ReturnInnerTxnOnly variable name does not really represent what this is doing--it makes it much harder to reason about these conditions.
Let's update the name on the develop branch--avoiding a bunch of refactoring on this hotfix branch seems more important.
I've updated the most recent patch to represent what we want, which is that if neither of these options is enabled we will fetch root.txn and also if neither is enabled we'll add the JOIN which references the intra index.
#1395 Shows more details.
At a high level, the SQL used to fetch transactions for the
/v2/blocksendpoint balloons the amount of memory usage when the block contains large numbers of inner transactions. These inner transactions end up getting discarded anyways after the query returns and is deserialized. This patch improves the query and memory usage by only fetching root transactions to begin with.Test Plan
Ran benchmarks from #1396 to get memory usage data. The result of a block containing 250 transactions, each of which has 250 inner transactions, w/ a
MaxTransactionsLimitset to the default of 1000 shows pre-patch memory usage around 9GB, and post-patch memory usage around 2GB.Before patch:
After patch: