Write into txn and txn_participation tables in parallel with other import procedures
#805
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #805 +/- ##
===========================================
+ Coverage 58.98% 59.10% +0.12%
===========================================
Files 30 32 +2
Lines 4084 4101 +17
===========================================
+ Hits 2409 2424 +15
- Misses 1378 1379 +1
- Partials 297 298 +1
Continue to review full report at Codecov.
|
|
@tolikzinovyev Could you move things back to |
I didn't change any code there, only moved it. |
| f := func(tx pgx.Tx) error { | ||
| return writer.AddTransactionParticipation(block, tx) | ||
| } | ||
| err0 = db.txWithRetry(serializable, f) |
There was a problem hiding this comment.
The nested tx with retry is a little strange. Could you call writer.AddTransactionParticipation directly?
There was a problem hiding this comment.
Why is this in a go routine, doesn't the evaluator need the txn participation in order to prefetch accounts?
There was a problem hiding this comment.
It would be more strange to retry the main transaction if this one fails.
Prefetching happens separately 15 lines below.
There was a problem hiding this comment.
This definitely compromises the "Writer" interface a bit, which is causing the AddBlock function to become more and more complicated. Previously it was a nice abstraction allowing this function to mostly concern itself with the evaluator, now it's tied up with subtle schema/query optimizations.
How are you testing the performance changes?
|
I agree that the interface is not pretty anymore. Suggestions are welcome. I run |
|
|
||
| return nil | ||
| return w.AddBlock(&block, block.Payset, ledgercore.StateDelta{}) |
There was a problem hiding this comment.
Can this can use db.AddBlock instead of w.AddBlock to simplify the test?
There was a problem hiding this comment.
it is your test, why did you use writer? :)
There was a problem hiding this comment.
I was probably trying to reduce the scope of the unit test, but now that it requires more implementation details from db.AddBlock that makes less sense.
Could you switch it to db.AddBlock?
|
@tolikzinovyev could you add more comments to |
|
Hi, what happens if connection to postgres is lost (or app crashes) while writing to |
The transactions writing to You can study the changes in |
|
Hi @tolikzinovyev excellent. Thanks for the feedback. |
Summary
AddTransactions()andAddTransactionParticipation()are taken out of the writer struct and made free functions, so that they can be called with a different database transaction in a different goroutine. This code performs at 9200 TPS in my tests. The current develop branch performs at 7000 TPS.AddTransactions()andAddTransactionParticipation()are called before the main db transaction commits, sotxnandtxn_participationtables can only be ahead of all other state. Extra care was taken to ensure that this desynchronization is not a problem. First, ifidb.AddBlock()sees postgres unique violation error when writing totxnandtxn_participationtables, it ignores this error since it means that these tables are ahead. Second, we need to make sure that Indexer query code works fine. At the moment onlyidb.Transactions()uses those two tables. Since they are JOINed with theblock_headertable, rows intxnandtxn_participationthat are ahead will be filtered out. In the future, if we need to add more queries that use these two tables, we could use database views such asSELECT * FROM txn WHERE round < $1.Closes #795.
Test Plan
Added a test that checks that
idb.AddBlock()runs successfully whentxnandtxn_participationare ahead.Added a test checking that
idb.AddBlock()does callAddTransactionParticipation().Added a test checking that when
txnandtxn_participationtables are ahead,idb.Transactions()behaves as if they were not.