Skip to content
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

Write transactions earlier if possible #807

Merged
merged 3 commits into from
Dec 10, 2021
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 29 additions & 18 deletions idb/postgres/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,16 +296,32 @@ func (db *IndexerDb) AddBlock(block *bookkeeping.Block) error {
return fmt.Errorf("AddBlock() err: %w", err)
}
} else {
proto, ok := config.Consensus[block.BlockHeader.CurrentProtocol]
if !ok {
return fmt.Errorf(
"AddBlock() cannot find proto version %s", block.BlockHeader.CurrentProtocol)
}
protoChanged := !proto.EnableAssetCloseAmount
proto.EnableAssetCloseAmount = true

var wg sync.WaitGroup
defer wg.Wait()

// Write transaction participation in a parallel db transaction.
// Write transaction participation and possibly transactions in a parallel db
// transaction. If `proto.EnableAssetCloseAmount` is already true, we can start
// writing transactions contained in the block early.
var err0 error
wg.Add(1)
go func() {
defer wg.Done()

f := func(tx pgx.Tx) error {
if !protoChanged {
err := writer.AddTransactions(block, block.Payset, tx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want want this in a separate txWithRetry so that it is written in parallel to AddTransactionParticipation?

I like this change but am not sure we need it right now. At the very least I think it's good to add a comment below near AddTransactions, something along the lines of:

if proto.EnableAssetCloseAmount was already true, the transactions can be loaded with block.Payset instead of modifiedTransactions while the evaluator is running

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, writing transactions and transaction participation together takes less time then eval + writing other state, so we don't need more parallelism; it would probably hurt.

I will add a comment.

if err != nil {
return err
}
}
return writer.AddTransactionParticipation(block, tx)
}
err0 = db.txWithRetry(serializable, f)
Expand All @@ -318,13 +334,6 @@ func (db *IndexerDb) AddBlock(block *bookkeeping.Block) error {
}
defer ledgerForEval.Close()

proto, ok := config.Consensus[block.BlockHeader.CurrentProtocol]
if !ok {
return fmt.Errorf(
"AddBlock() cannot find proto version %s", block.BlockHeader.CurrentProtocol)
}
proto.EnableAssetCloseAmount = true

resources, err := prepareEvalResources(&ledgerForEval, block)
if err != nil {
return fmt.Errorf("AddBlock() eval err: %w", err)
Expand All @@ -338,17 +347,19 @@ func (db *IndexerDb) AddBlock(block *bookkeeping.Block) error {
}
metrics.PostgresEvalTimeSeconds.Observe(time.Since(start).Seconds())

// Write transactions in a parallel db transaction.
var err1 error
wg.Add(1)
go func() {
defer wg.Done()

f := func(tx pgx.Tx) error {
return writer.AddTransactions(block, modifiedTxns, tx)
}
err1 = db.txWithRetry(serializable, f)
}()
if protoChanged {
tolikzinovyev marked this conversation as resolved.
Show resolved Hide resolved
// Write transactions in a parallel db transaction.
wg.Add(1)
go func() {
defer wg.Done()

f := func(tx pgx.Tx) error {
return writer.AddTransactions(block, modifiedTxns, tx)
}
err1 = db.txWithRetry(serializable, f)
}()
}

err = w.AddBlock(block, modifiedTxns, delta)
if err != nil {
Expand Down