Tx replication sometimes fails. #1650 #1651
Conversation
There was a problem hiding this comment.
You ensured the transaction has been succesfully written before ingesting it only when writting the previous transactions of the chain. But in there is 2 others call of write_transaction that need the same check.
Also this PR could be merge into develop for the next release. It do not needs to be in 1.7.0
lib/archethic/db/embedded_impl.ex
Outdated
| ChainWriter.append_transaction(genesis_address, tx) | ||
|
|
||
| # Delete IO transaction if it exists as it is now stored as a chain | ||
| delete_io_transaction(tx.address) |
There was a problem hiding this comment.
Calling this function after the append_transaction will ignore the return of the ChainWriter, so the function will never return {:error, :transaction_already_exists}
You could also updated the spec of the function
| @@ -36,21 +36,25 @@ defmodule Archethic.DB.EmbeddedImpl.ChainWriter do | |||
| """ | |||
| @spec write_io_transaction(Transaction.t(), String.t()) :: :ok | |||
There was a problem hiding this comment.
You can update the spec of the function since it has a new return
| @@ -154,23 +158,27 @@ defmodule Archethic.DB.EmbeddedImpl.ChainWriter do | |||
| end | |||
|
|
|||
| defp write_transaction(genesis_address, tx, db_path) do | |||
| start = System.monotonic_time() | |||
| if ChainIndex.transaction_exists?(tx.address, db_path) do | |||
There was a problem hiding this comment.
Suggestion:
This control could be done in the handle_call of append_tx. So function write_transaction only write and do not perform check.
lib/archethic/replication.ex
Outdated
|
|
||
| opts = Keyword.delete(ingest_opts, :resolved_addresses) | ||
| if ingest?, do: ingest_transaction(tx, genesis_address, opts) | ||
| case TransactionChain.write_transaction(tx) do |
There was a problem hiding this comment.
Suggestion:
Since we don't need a specific return from the lambda function (Stream.each always return :ok), you could simplify the function like
if TransactionChain.write_transaction(tx) == :ok do
# There is some case where a transaction is not replicated while it should
# because of some latency or network issue. So when we replicate a past chain
# we also ingest the transaction if we are storage node of it
ingest? =
Transaction.network_type?(type) or
Election.chain_storage_node?(address, first_node_key, download_nodes) or
Election.chain_storage_node?(genesis_address, first_node_key, download_nodes)
opts = Keyword.delete(ingest_opts, :resolved_addresses)
if ingest?, do: ingest_transaction(tx, genesis_address, opts)
end
lib/archethic/transaction_chain.ex
Outdated
| @@ -1128,7 +1128,7 @@ defmodule Archethic.TransactionChain do | |||
| Persist only one transaction | |||
| """ | |||
| @spec write_transaction(transaction :: Transaction.t(), storage_location :: DB.storage_type()) :: | |||
| :ok | |||
| :ok | {:error, :reason} | |||
There was a problem hiding this comment.
The function will never return the atom :reason as error details. The right return spec sould be {:error, :transaction_already_exists}
7465d23 to
b49398c
Compare
b49398c to
fde4650
Compare
Description
When replicating transactions, checks if Tx is already present before writing it.
Check is performed in
ChainWriter.writeto ensure they are sequentials.Fixes #1650