Restart the submission interpretation in case of a race [DPP-737] #11552
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gerolf-da
force-pushed
the
gerolf_retry-on-ledger-time-lookup-failure
branch
2 times, most recently
from
6c81d8b to
c73e338
Compare
mziolekda
reviewed
Nov 4, 2021
...gration-api/src/main/scala/platform/apiserver/execution/LedgerTimeAwareCommandExecutor.scala
Show resolved
Hide resolved
mziolekda
reviewed
Nov 4, 2021
...gration-api/src/main/scala/platform/apiserver/execution/LedgerTimeAwareCommandExecutor.scala
Outdated
Show resolved
Hide resolved
mziolekda
reviewed
Nov 4, 2021
...ration-api/src/main/scala/platform/store/backend/common/ContractStorageBackendTemplate.scala
Show resolved
Hide resolved
mziolekda
changed the title
Command interpretation happens in two stages: 1) Daml interpretation 2) Determine a suitable ledger effective time The race can happen in the following situation: In 1) a contract key K is resolved to contract C1. Between 1) and 2), a transaction is stored on the participant that archives C1, but creates C2 with the same contract key K. In 2) the ledger api server tries to lookup the ledger effective time for all input contracts to the transaction. If it doesn't find one of the input contracts at all, it can conclude that the transaction wouldn't be accepted by the ledger anyway. The behavior before this patch was to simply abort command interpretation and return a rather cryptic error to the user. With this patch, the ledger api server restarts the command interpretation. If the "missing contract" was an explicit input to the command (e.g. as the contract for the exercise or as an argument to an exercise), then this command will be rejected because the contract is now archived. If the contract ID was determined via a contract key lookup, then restarting the interpretation will result in either a negative lookup or a different contract ID for the new contract under this contract key. CHANGELOG_BEGIN [Ledger API] Retry the interpretation of a command in case of a race with other transactions. This fix drastically reduces the likelihood of the error "Could not find a suitable ledger time after 0 retries". CHANGELOG_END
I thought we had types that the compiler checks at compile time :( CHANGELOG_BEGIN CHANGELOG_END
gerolf-da
force-pushed
the
gerolf_retry-on-ledger-time-lookup-failure
branch
from
7d140ad to
a9c351b
Compare
tudor-da
reviewed
Nov 8, 2021
...gration-api/src/main/scala/platform/apiserver/execution/LedgerTimeAwareCommandExecutor.scala
Show resolved
Hide resolved
mziolekda
reviewed
Nov 8, 2021
| for { | ||
| let <- letE | ||
| acc <- acc | ||
| } yield acc.map(acc => if (let.isAfter(acc)) let else acc) |
There was a problem hiding this comment.
Aren't we supposed to collect all the missing ids into the accumulator's left?
mziolekda
reviewed
Nov 8, 2021
| ) | ||
| } | ||
|
|
||
| "retry if the contract's LET is in the future and then retry if the contract is missing" in { |
There was a problem hiding this comment.
Nice, this is precisely what I was looking for
7 tasks
tudor-da
reviewed
Nov 8, 2021
| Future.fromTry(Try(this.synchronized { | ||
| contractIds | ||
| .foldLeft[Either[Set[ContractId], Option[Instant]]](Right(Some(Instant.MIN)))((acc, id) => { | ||
| val letE = acs.activeContracts.get(id).map(c => Right(c.let)).getOrElse(Left(Set(id))) |
There was a problem hiding this comment.
If contractIds is empty, this function returns Right(Some(Instant.MIN)) - shouldn't it be Right(None)?
There was a problem hiding this comment.
I see this is how it was before as well. Fine not to change then.
7 tasks
|
Setting it back to draft. We'd rather go with the exception-based implementation in #11579. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Command interpretation happens in two stages:
The race can happen in the following situation:
In 1) a contract key K is resolved to contract C1.
Between 1) and 2), a transaction is stored on the participant that
archives C1, but creates C2 with the same contract key K.
In 2) the ledger api server tries to lookup the ledger effective time
for all input contracts to the transaction. If it doesn't find one of
the input contracts at all, it can conclude that the transaction
wouldn't be accepted by the ledger anyway.
The behavior before this patch was to simply abort command
interpretation and return a rather cryptic error to the user.
With this patch, the ledger api server restarts the command
interpretation.
If the "missing contract" was an explicit input to the
command (e.g. as the contract for the exercise or as an argument to an
exercise), then this command will be rejected because the contract is
now archived.
If the contract ID was determined via a contract key lookup, then
restarting the interpretation will result in either a negative lookup or
a different contract ID for the new contract under this contract key.
CHANGELOG_BEGIN
[Ledger API] Retry the interpretation of a command in case of a race
with other transactions. This fix drastically reduces the likelihood of the error
"Could not find a suitable ledger time after 0 retries".
CHANGELOG_END
Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.