Deadlocks caused by bad ordering of address_current_token_balances upserts

[goodsoft]

Despite fixes in #1296 and #1320 deadlocks still occur during block synchronization:

2019-03-04T21:29:30.422 count=100 [error] Task #PID<0.31248.0> started from Indexer.TokenBalance.Fetcher terminating
** (Postgrex.Error) ERROR 40P01 (deadlock_detected) deadlock detected

    hint: See server log for query details.

Process 20005 waits for ShareLock on transaction 238640; blocked by process 19990.
Process 19990 waits for ShareLock on transaction 238669; blocked by process 20005.
    (ecto_sql) lib/ecto/adapters/sql.ex:620: Ecto.Adapters.SQL.raise_sql_call_error/1
    (ecto_sql) lib/ecto/adapters/sql.ex:529: Ecto.Adapters.SQL.insert_all/8
    (ecto) lib/ecto/repo/schema.ex:56: Ecto.Repo.Schema.do_insert_all/6
    (explorer) lib/explorer/repo.ex:45: anonymous fn/5 in Explorer.Repo.safe_insert_all/3
    (elixir) lib/enum.ex:1940: Enum."-reduce/3-lists^foldl/2-0-"/3
    (explorer) lib/explorer/chain/import.ex:285: Explorer.Chain.Import.insert_changes_list/3

    (explorer) lib/explorer/logger.ex:14: Explorer.Logger.metadata/2

Apparently, realtime block fetcher upserts into address_current_token_balances table twice in the span of a huge transaction:

BEGIN
INSERT INTO "address_coin_balances"
INSERT INTO transaction_forks
UPDATE "transactions"
UPDATE "blocks"
DELETE FROM "address_token_balances"
DELETE FROM "address_current_token_balances"
INSERT INTO address_current_token_balances      <-- first
UPDATE tokens
DELETE FROM "block_rewards"
INSERT INTO "blocks"
UPDATE "block_second_degree_relations"
UPDATE "internal_transactions"
INSERT INTO "block_rewards"
INSERT INTO "transactions"
INSERT INTO "internal_transactions"
UPDATE "transactions"
INSERT INTO "logs"
INSERT INTO "tokens"
INSERT INTO "token_transfers"
SELECT FROM "address_current_token_balances"
INSERT INTO "address_current_token_balances"    <-- second
UPDATE tokens
INSERT INTO "address_token_balances"
COMMIT

The offending transaction hits the deadlock while upserting to the same table during the second upsert above:

BEGIN
SELECT FROM "address_current_token_balances"
INSERT INTO "address_current_token_balances"    <-- oops

I discovered token balance updating in two places in the codebase:

1. https://github.com/poanetwork/blockscout/blob/master/apps/explorer/lib/explorer/chain/import/runner/address/current_token_balances.ex#L112-L135
2. https://github.com/poanetwork/blockscout/blob/master/apps/explorer/lib/explorer/chain/import/runner/blocks.ex#L75-L92

This seems to be causing unordered upserts and deadlocks.

Before going on and removing one of these code blocks, I would like to understand the purpose of this duplication and figure out, which one should actually be removed.

A comment from @KronicDeth, who is apparently the author of both code blocks would be very much appreciated.

[KronicDeth]

derive_address_current_token_balances is a consensus loss function. It is needed to keep the address_current_token_balances accurate when their block changes consensus. If you remove it the address_current_token_balances will reflect non-consensus when the highest block number for a given {adress, token} as recorded in address_current_token_balances becomes non-consensus.

address_current_token_balances is the normal "insert into the runner's table" step. It needs to be there for when there are address_current_token_balances params passed in. Deleting this step would mean you couldn't write new address_current_token_balances.

You can't delete either step.

What you could do is look at the params address_current_token_balances will try to insert and don't
delete them in : delete_address_current_token_balances or derive them derive_address_current_token_balances for those {address, token} pairs as you know they'll be fixed later in address_current_token_balances.

[goodsoft]

Thanks for the elaborate answer!

Just a quick thought before diving back into the code: splitting the transaction into one with consensus loss and another one with the actual block import would be an easy solution to the deadlock problem.
The question is: would it break the database consistency?

[KronicDeth]

You have too many writers to guarantee ordering if you split the transactions. You can't ensure that you take away consensus before adding it back with the normal blocks.

[goodsoft]

Fixed via #1724