From 2e3627d5367d734e9b7821d2c9860bc773f98f52 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 3 Oct 2018 10:17:47 -0500 Subject: [PATCH] Don't run imports for empty params --- apps/explorer/lib/explorer/chain/import.ex | 41 +++++-- .../test/explorer/chain/import_test.exs | 109 +++++++++++++++--- .../lib/indexer/block/catchup/fetcher.ex | 28 ++--- .../lib/indexer/block/realtime/fetcher.ex | 12 +- .../indexer/internal_transaction/fetcher.ex | 5 +- .../test/indexer/block/fetcher_test.exs | 4 +- .../indexer/block/realtime/fetcher_test.exs | 1 - 7 files changed, 145 insertions(+), 55 deletions(-) diff --git a/apps/explorer/lib/explorer/chain/import.ex b/apps/explorer/lib/explorer/chain/import.ex index 682fd1f6b9..ec253df704 100644 --- a/apps/explorer/lib/explorer/chain/import.ex +++ b/apps/explorer/lib/explorer/chain/import.ex @@ -1194,20 +1194,39 @@ defmodule Explorer.Chain.Import do end defp import_options_to_changes_list_arguments_list(options) do - Enum.flat_map(@import_option_key_to_ecto_schema_module, fn {option_key, ecto_schema_module} -> - case Map.fetch(options, option_key) do - {:ok, option_value} when is_map(option_value) -> + Enum.flat_map( + @import_option_key_to_ecto_schema_module, + &import_options_to_changes_list_arguments_list_flat_mapper(options, &1) + ) + end + + defp import_options_to_changes_list_arguments_list_flat_mapper(options, {option_key, ecto_schema_module}) do + case Map.fetch(options, option_key) do + {:ok, option_value} -> + import_option_to_changes_list_arguments_list_flat_mapper(option_value, ecto_schema_module) + + :error -> + [] + end + end + + defp import_option_to_changes_list_arguments_list_flat_mapper(%{params: params} = option_value, ecto_schema_module) do + # Use `Enum.empty?` instead of `[_ | _]` as params are allowed to be any collection of maps + case Enum.empty?(params) do + false -> + [ [ - [ - Map.fetch!(option_value, :params), - [for: ecto_schema_module, with: Map.get(option_value, :with, :changeset)] - ] + params, + [for: ecto_schema_module, with: Map.get(option_value, :with, :changeset)] ] + ] - :error -> - [] - end - end) + # filter out empty params as early as possible, so that later stages don't need to deal with empty params + # leading to selecting all rows because they produce no where conditions as happened in + # https://github.com/poanetwork/blockscout/issues/850 + true -> + [] + end end defp import_transaction(multi, options) when is_map(options) do diff --git a/apps/explorer/test/explorer/chain/import_test.exs b/apps/explorer/test/explorer/chain/import_test.exs index 3a40a7b0ed..5e07e4831d 100644 --- a/apps/explorer/test/explorer/chain/import_test.exs +++ b/apps/explorer/test/explorer/chain/import_test.exs @@ -1461,52 +1461,124 @@ defmodule Explorer.Chain.ImportTest do end test "timeouts can be overridden" do - assert {:ok, _} = + miner_hash = address_hash() + uncle_miner_hash = address_hash() + block_number = 0 + block_hash = block_hash() + uncle_hash = block_hash() + from_address_hash = address_hash() + to_address_hash = address_hash() + transaction_hash = transaction_hash() + token_contract_address_hash = address_hash() + + assert {:ok, + %{ + addresses: _, + balances: _, + blocks: _, + block_second_degree_relations: _, + internal_transactions: _, + logs: _, + token_transfers: _, + tokens: _, + transactions: _, + transaction_forks: _, + token_balances: _ + }} = Import.all(%{ addresses: %{ - params: [], + params: [ + %{hash: miner_hash}, + %{hash: uncle_miner_hash}, + %{hash: to_address_hash}, + %{hash: from_address_hash}, + %{hash: token_contract_address_hash} + ], timeout: 1 }, balances: %{ - params: [], + params: [ + %{address_hash: miner_hash, block_number: block_number, value: nil}, + %{address_hash: uncle_miner_hash, block_number: block_number, value: nil} + ], timeout: 1 }, blocks: %{ - params: [], + params: [ + params_for(:block, hash: block_hash, consensus: true, miner_hash: miner_hash, number: block_number), + params_for(:block, + hash: uncle_hash, + consensus: false, + miner_hash: uncle_miner_hash, + number: block_number + ) + ], timeout: 1 }, block_second_degree_relations: %{ - params: [], + params: [%{nephew_hash: block_hash, uncle_hash: uncle_hash}], timeout: 1 }, internal_transactions: %{ - params: [], + params: [ + params_for(:internal_transaction, + transaction_hash: transaction_hash, + index: 0, + from_address_hash: from_address_hash, + to_address_hash: to_address_hash + ) + ], timeout: 1 }, logs: %{ - params: [], + params: [params_for(:log, transaction_hash: transaction_hash, address_hash: miner_hash)], timeout: 1 }, token_transfers: %{ - params: [], + params: [ + params_for(:token_transfer, + from_address_hash: from_address_hash, + to_address_hash: to_address_hash, + token_contract_address_hash: token_contract_address_hash, + transaction_hash: transaction_hash + ) + ], timeout: 1 }, tokens: %{ - params: [], + params: [params_for(:token, contract_address_hash: token_contract_address_hash)], on_conflict: :replace_all, timeout: 1 }, transactions: %{ - params: [], + params: [ + params_for(:transaction, + hash: transaction_hash, + block_hash: block_hash, + block_number: block_number, + index: 0, + from_address_hash: from_address_hash, + to_address_hash: to_address_hash, + gas_used: 0, + cumulative_gas_used: 0 + ) + ], on_conflict: :replace_all, timeout: 1 }, transaction_forks: %{ - params: [], + params: [%{uncle_hash: uncle_hash, hash: transaction_hash, index: 0}], timeout: 1 }, token_balances: %{ - params: [], + params: [ + params_for( + :token_balance, + address_hash: to_address_hash, + token_contract_address_hash: token_contract_address_hash, + block_number: block_number + ) + ], timeout: 1 } }) @@ -1631,15 +1703,14 @@ defmodule Explorer.Chain.ImportTest do end # https://github.com/poanetwork/blockscout/issues/850 regression test - test "derive_transaction_forks does not try to fork pending transactions when there are no blocks" do + test "derive_transaction_forks does not run when there are no blocks" do _pending_transaction = insert(:transaction) - {:ok, %{derive_transaction_forks: []}} = - Import.all(%{ - blocks: %{ - params: [] - } - }) + assert Import.all(%{ + blocks: %{ + params: [] + } + }) == {:ok, %{}} end end end diff --git a/apps/indexer/lib/indexer/block/catchup/fetcher.ex b/apps/indexer/lib/indexer/block/catchup/fetcher.ex index be45a3080f..c35c7c670e 100644 --- a/apps/indexer/lib/indexer/block/catchup/fetcher.ex +++ b/apps/indexer/lib/indexer/block/catchup/fetcher.ex @@ -108,12 +108,12 @@ defmodule Indexer.Block.Catchup.Fetcher do {async_import_remaining_block_data_options, chain_import_options} = Map.split(options, @async_import_remaining_block_data_options) - with {:ok, results} = ok <- + with {:ok, imported} = ok <- chain_import_options |> put_in([:blocks, :params, Access.all(), :consensus], true) |> Chain.import() do async_import_remaining_block_data( - results, + imported, async_import_remaining_block_data_options ) @@ -122,39 +122,39 @@ defmodule Indexer.Block.Catchup.Fetcher do end defp async_import_remaining_block_data( - %{ - block_second_degree_relations: block_second_degree_relations, - transactions: transaction_hashes, - addresses: address_hashes, - tokens: tokens, - token_balances: token_balances - }, + imported, %{ address_hash_to_fetched_balance_block_number: address_hash_to_block_number, transaction_hash_to_block_number: transaction_hash_to_block_number } ) do - address_hashes + imported + |> Map.get(:addresses, []) |> Enum.map(fn address_hash -> block_number = Map.fetch!(address_hash_to_block_number, to_string(address_hash)) %{address_hash: address_hash, block_number: block_number} end) |> CoinBalance.Fetcher.async_fetch_balances() - transaction_hashes + imported + |> Map.get(:transactions, []) |> Enum.map(fn transaction_hash -> block_number = Map.fetch!(transaction_hash_to_block_number, to_string(transaction_hash)) %{block_number: block_number, hash: transaction_hash} end) |> InternalTransaction.Fetcher.async_fetch(10_000) - tokens + imported + |> Map.get(:tokens, []) |> Enum.map(& &1.contract_address_hash) |> Token.Fetcher.async_fetch() - TokenBalance.Fetcher.async_fetch(token_balances) + imported + |> Map.get(:token_balances, []) + |> TokenBalance.Fetcher.async_fetch() - block_second_degree_relations + imported + |> Map.get(:block_second_degree_relations, []) |> Enum.map(& &1.uncle_hash) |> Block.Uncle.Fetcher.async_fetch_blocks() end diff --git a/apps/indexer/lib/indexer/block/realtime/fetcher.ex b/apps/indexer/lib/indexer/block/realtime/fetcher.ex index 282ca0ceed..e87a73c16d 100644 --- a/apps/indexer/lib/indexer/block/realtime/fetcher.ex +++ b/apps/indexer/lib/indexer/block/realtime/fetcher.ex @@ -108,9 +108,9 @@ defmodule Indexer.Block.Realtime.Fetcher do |> put_in([Access.key(:balances, %{}), :params], balances_params) |> put_in([Access.key(:internal_transactions, %{}), :params], internal_transactions_params) |> put_in([Access.key(:token_balances), :params], token_balances), - {:ok, results} = ok <- Chain.import(chain_import_options) do + {:ok, imported} = ok <- Chain.import(chain_import_options) do TokenBalances.log_fetching_errors(__MODULE__, token_balances) - async_import_remaining_block_data(results) + async_import_remaining_block_data(imported) ok end end @@ -200,12 +200,14 @@ defmodule Indexer.Block.Realtime.Fetcher do Enum.any?(changesets, &(Map.get(&1, :message) == "Unknown block number")) end - defp async_import_remaining_block_data(%{block_second_degree_relations: block_second_degree_relations, tokens: tokens}) do - tokens + defp async_import_remaining_block_data(imported) do + imported + |> Map.get(:tokens, []) |> Enum.map(& &1.contract_address_hash) |> Token.Fetcher.async_fetch() - block_second_degree_relations + imported + |> Map.get(:block_second_degree_relations, []) |> Enum.map(& &1.uncle_hash) |> Block.Uncle.Fetcher.async_fetch_blocks() end diff --git a/apps/indexer/lib/indexer/internal_transaction/fetcher.ex b/apps/indexer/lib/indexer/internal_transaction/fetcher.ex index e422122175..898ae5ee7f 100644 --- a/apps/indexer/lib/indexer/internal_transaction/fetcher.ex +++ b/apps/indexer/lib/indexer/internal_transaction/fetcher.ex @@ -99,13 +99,14 @@ defmodule Indexer.InternalTransaction.Fetcher do {hash, block_number} end) - with {:ok, %{addresses: address_hashes}} <- + with {:ok, imported} <- Chain.import(%{ addresses: %{params: addresses_params}, internal_transactions: %{params: internal_transactions_params}, timeout: :infinity }) do - address_hashes + imported + |> Map.get(:addresses, []) |> Enum.map(fn address_hash -> block_number = Map.fetch!(address_hash_to_block_number, to_string(address_hash)) %{address_hash: address_hash, block_number: block_number} diff --git a/apps/indexer/test/indexer/block/fetcher_test.exs b/apps/indexer/test/indexer/block/fetcher_test.exs index 9bd16e0fa8..a68c28218f 100644 --- a/apps/indexer/test/indexer/block/fetcher_test.exs +++ b/apps/indexer/test/indexer/block/fetcher_test.exs @@ -213,9 +213,7 @@ defmodule Indexer.Block.FetcherTest do assert {:ok, {%{ addresses: [%Address{hash: ^address_hash}], - blocks: [%Chain.Block{hash: ^block_hash}], - logs: [], - transactions: [] + blocks: [%Chain.Block{hash: ^block_hash}] }, :more}} = result wait_for_tasks(InternalTransaction.Fetcher) diff --git a/apps/indexer/test/indexer/block/realtime/fetcher_test.exs b/apps/indexer/test/indexer/block/realtime/fetcher_test.exs index 68dbdfb741..2f3a4a6bbc 100644 --- a/apps/indexer/test/indexer/block/realtime/fetcher_test.exs +++ b/apps/indexer/test/indexer/block/realtime/fetcher_test.exs @@ -402,7 +402,6 @@ defmodule Indexer.Block.Realtime.FetcherTest do %{index: 4, transaction_hash: transaction_hash}, %{index: 5, transaction_hash: transaction_hash} ], - logs: [], transactions: [transaction_hash] }, :more}} = Indexer.Block.Fetcher.fetch_and_import_range(block_fetcher, 3_946_079..3_946_080) end