From 757e7ada044424b79854393eaa074a15e47ed35e Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 10:28:53 -0500 Subject: [PATCH 1/8] Cancel transaction when exiting AccountAtRound. --- accounting/rewind.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/accounting/rewind.go b/accounting/rewind.go index 82dabb110..d03f49cb3 100644 --- a/accounting/rewind.go +++ b/accounting/rewind.go @@ -99,7 +99,13 @@ func AccountAtRound(ctx context.Context, account models.Account, round uint64, d MinRound: round + 1, MaxRound: account.Round, } + ctx, cf := context.WithCancel(ctx) txns, r := db.Transactions(ctx, tf) + defer func() { + cf() + for range txns { + } + }() if r < account.Round { err = ConsistencyError{fmt.Sprintf("queried round r: %d < account.Round: %d", r, account.Round)} return From 93149dbd2589c21a5065f24af2f251ed703f657b Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 12:33:48 -0500 Subject: [PATCH 2/8] Split function in two to avoid multiple defers. --- accounting/rewind.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/accounting/rewind.go b/accounting/rewind.go index d03f49cb3..5cfb09797 100644 --- a/accounting/rewind.go +++ b/accounting/rewind.go @@ -99,17 +99,21 @@ func AccountAtRound(ctx context.Context, account models.Account, round uint64, d MinRound: round + 1, MaxRound: account.Round, } - ctx, cf := context.WithCancel(ctx) - txns, r := db.Transactions(ctx, tf) - defer func() { - cf() - for range txns { - } - }() + ctx2, cf := context.WithCancel(ctx) + defer cf() + txns, r := db.Transactions(ctx2, tf) if r < account.Round { err = ConsistencyError{fmt.Sprintf("queried round r: %d < account.Round: %d", r, account.Round)} return } + return processTransactions(account, addr, round, txns) +} + +func processTransactions(account models.Account, addr basics.Address, round uint64, txns <-chan idb.TxnRow) (acct models.Account, err error) { + defer func() { + for range txns { + } + }() txcount := 0 for txnrow := range txns { if txnrow.Error != nil { From 815a62d5b96c368852c4ef8e887bd51dad686d46 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 12:44:31 -0500 Subject: [PATCH 3/8] Call cf twice instead of splitting function, drain accounts channel. --- api/handlers.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/handlers.go b/api/handlers.go index 0137e68e0..ad2d07903 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -879,6 +879,12 @@ func (si *ServerImplementation) fetchAccounts(ctx context.Context, options idb.A var accountchan <-chan idb.AccountRow accountchan, round = si.db.GetAccounts(ctx, options) + // Make sure accountchan is empty at the end of processing. + defer func() { + for range accountchan { + } + }() + if (atRound != nil) && (*atRound > round) { return fmt.Errorf("%s: the requested round %d > the current round %d", errRewindingAccount, *atRound, round) From dfe7967cd8f2d5f364c75748e8fb4bdd931d13e8 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 15:27:49 -0500 Subject: [PATCH 4/8] Revert split function change. --- accounting/rewind.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/accounting/rewind.go b/accounting/rewind.go index 5cfb09797..abc195c53 100644 --- a/accounting/rewind.go +++ b/accounting/rewind.go @@ -102,18 +102,15 @@ func AccountAtRound(ctx context.Context, account models.Account, round uint64, d ctx2, cf := context.WithCancel(ctx) defer cf() txns, r := db.Transactions(ctx2, tf) - if r < account.Round { - err = ConsistencyError{fmt.Sprintf("queried round r: %d < account.Round: %d", r, account.Round)} - return - } - return processTransactions(account, addr, round, txns) -} - -func processTransactions(account models.Account, addr basics.Address, round uint64, txns <-chan idb.TxnRow) (acct models.Account, err error) { defer func() { + cf() for range txns { } }() + if r < account.Round { + err = ConsistencyError{fmt.Sprintf("queried round r: %d < account.Round: %d", r, account.Round)} + return + } txcount := 0 for txnrow := range txns { if txnrow.Error != nil { From aef1602979d5fab5ebc6fcad19b252b874f3e19f Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 16:31:33 -0500 Subject: [PATCH 5/8] Extend circleCI timeout. --- .circleci/config.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2f3b07ea8..97de5ee71 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -99,6 +99,7 @@ commands: command: go get -u golang.org/x/lint/golint run_tests: + no_output_timeout: 15m steps: - run: test -z `go fmt ./...` - run: make lint @@ -110,6 +111,7 @@ commands: - run: make e2e run_tests_nightly: + no_output_timeout: 15m steps: - run: test -z `go fmt ./...` - run: make lint From 973dbec15b8a3a12860a309e040bdf9525f2e1d9 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 17:18:47 -0500 Subject: [PATCH 6/8] Extend circleCI timeout. --- .circleci/config.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 97de5ee71..c4a692dfa 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -99,25 +99,27 @@ commands: command: go get -u golang.org/x/lint/golint run_tests: - no_output_timeout: 15m steps: - run: test -z `go fmt ./...` - run: make lint - run: make check - run: make integration - - run: make test + - run: + command: make test + no_output_timeout: 15m - run: make test-generate - run: make fakepackage - run: make e2e run_tests_nightly: - no_output_timeout: 15m steps: - run: test -z `go fmt ./...` - run: make lint - run: make check - run: make integration - - run: make test + - run: + command: make test + no_output_timeout: 15m - run: make test-generate - run: make fakepackage - run: make e2e From 95d406653c6c7731934f4c6b649c29ff4369cd87 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 19:55:44 -0500 Subject: [PATCH 7/8] Fix test. --- accounting/rewind_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/accounting/rewind_test.go b/accounting/rewind_test.go index 00bd796cf..dbd79ed3b 100644 --- a/accounting/rewind_test.go +++ b/accounting/rewind_test.go @@ -67,10 +67,12 @@ func TestStaleTransactions1(t *testing.T) { Round: 8, } - var outCh <-chan idb.TxnRow + ch := make(chan idb.TxnRow) + var outCh <-chan idb.TxnRow = ch + close(ch) db := &mocks.IndexerDb{} - db.On("GetSpecialAccounts").Return(transactions.SpecialAddresses{}, nil) + db.On("GetSpecialAccounts", mock.Anything).Return(transactions.SpecialAddresses{}, nil) db.On("Transactions", mock.Anything, mock.Anything).Return(outCh, uint64(7)).Once() account, err := AccountAtRound(context.Background(), account, 6, db) From 1a5ea7d106d5c72aa7fe335c44bb4b8ad3bf4690 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 23 Feb 2022 20:04:27 -0500 Subject: [PATCH 8/8] Add comments. --- accounting/rewind.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/accounting/rewind.go b/accounting/rewind.go index abc195c53..dfa1290e0 100644 --- a/accounting/rewind.go +++ b/accounting/rewind.go @@ -100,8 +100,10 @@ func AccountAtRound(ctx context.Context, account models.Account, round uint64, d MaxRound: account.Round, } ctx2, cf := context.WithCancel(ctx) + // In case of a panic before the next defer, call cf() here. defer cf() txns, r := db.Transactions(ctx2, tf) + // In case of an error, make sure the context is cancelled, and the channel is cleaned up. defer func() { cf() for range txns {