Enable pruning in the sandbox-classic when the append-only schema is used [DPP-567] #10708
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
…used CHANGELOG_BEGIN - [Sandbox] - Participant pruning is enabled in the sandbox-classic when the append-only schema is used CHANGELOG_END
mziolekda
reviewed
Aug 31, 2021
nmarton-da
reviewed
Aug 31, 2021
| override def prune( | ||
| pruneUpToInclusive: Offset, | ||
| submissionId: Ref.SubmissionId, | ||
| pruneAllDivulgedContracts: Boolean, | ||
| ): CompletionStage[state.PruningResult] = | ||
| CompletableFuture.completedFuture(state.PruningResult.NotPruned(Status.UNIMPLEMENTED)) | ||
| CompletableFuture.completedFuture { | ||
| if (enablePruning) state.PruningResult.ParticipantPruned |
There was a problem hiding this comment.
this is not enabling pruning, just pretending to prune, no?
how is the conformance tests pass then?
am I missing something? 🤔
There was a problem hiding this comment.
If I understand correctly the ApiParticipantPruningService performs pruning in two steps:
- pruning the write service
- pruning the index db
For the sandbox-classic they are the same thing, so returning ParticipantPruned for the step 1. enables the step two.
@mziolekda can you confirm that this is correct?
There was a problem hiding this comment.
yupp, makes sense, a comment would be nice to clarify this behavior in the write service
There was a problem hiding this comment.
pruneAllDivulgedContracts is not meant to be supported for sandbox-classic because the backfiling lookup queries can start failing if used. Please return UNIMPLEMENTED if that flag is set. And please add a mention to the restriction in this method's doc/comments (e.g. Pruning of all divulged contracts is not supported on sandbox-classic)
tudor-da
reviewed
Aug 31, 2021
| ledger = MeteredLedger(ledger, metrics), | ||
| participantId = participantId, | ||
| timeProvider = timeProvider, | ||
| enablePruning = false, |
There was a problem hiding this comment.
Why can't we enable pruning for in-memory ledger?
There was a problem hiding this comment.
Maybe you can document it in the PR header why this one is left out
There was a problem hiding this comment.
It is a corer case that we should not spend any time debating at this point. It would have to go through another story if we wanted that support.
There was a problem hiding this comment.
fine by me. Just observed that there is no mention in the ticket or the PR regarding it.
kamil-da
commented
Aug 31, 2021
| @@ -724,7 +724,7 @@ private[testtool] final class ParticipantTestContext private[participant] ( | |||
| def prune( | |||
| pruneUpTo: LedgerOffset, | |||
| attempts: Int = 10, | |||
| pruneAllDivulgedContracts: Boolean = true, | |||
| pruneAllDivulgedContracts: Boolean = false, | |||
There was a problem hiding this comment.
There are only three test cases in ParticipantPruningIT that specifically test pruning divulged contracts. For these cases we enable pruneAllDivulgedContracts explicitly, leaving the flag disabled for all other cases.
tudor-da
approved these changes
Aug 31, 2021
azure-pipelines bot
pushed a commit
that referenced
this pull request
Sep 1, 2021
This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @aherrmann-da is in charge of this release. Commit log: ``` f058c2f DPP-368 clean up flags (#10711) 90ad24f [Divulgence pruning] Prune immediate divulgence [DPP-513] (#10691) 183934b Command dedup: add columns to completions in append-only schemas [KVL-1057] (#10652) 27c1333 LF: Drop outdated TODOs (#10725) 9be577a Enable pruning in the sandbox-classic when the append-only schema is used [DPP-567] (#10708) 9f072ae Ledger-API Conformance test for Contract ID V0 (#10717) 867547c DPP-368 enable append-only flag in sandbox (#10710) bdc511e [DPP-438] Change open-ended metric names into static ones (by removing partyName part) (#10706) 0c82006 [Divulgence pruning] Prune all divulged contracts only after migration offset [DPP-483] (#10661) 2555dbb Use soft references for values in the caches (#10715) 7fd5906 Add LedgerClientWithoutLedgerId next to the LedgerClient (#10681) 856c69c participant-integration-api: Increase a test timeout. (#10721) b86d07d remove non functioning oracle json array indices (#10720) f5e1756 sandbox-classic: Remove default parameters in `SqlLedger.Owner`. (#10718) 9ef3377 LF: Update specification with Contract ID Comparability check (#10703) e5c4734 enable JSON search index on Oracle by default (#10539) 1ded42f [DPP-418] Protect TLS keys - follow-up cleanup (#10696) 3fcd986 Introduce a new `Offset` format [KVL-1063] (#10668) a5781a6 update NOTICES file (#10714) 8985505 participant-integration-api: Use deadlines, not deduplication times, for expiring trackers. [KVL-1009] (#10704) 65025c2 sandbox-classic: Add ErrorInfo metadata for rejections. [KVL-1048] (#10707) 97bda3c LF: V1 Contract ID check in Preprocessor (#10687) c2f90ef Add CLI option to force disabling of participant deduplication (#10698) 6016633 Construct ParsedModule directly in Daml Repl (#10701) bbdf16a DPP-368 unhide append-only CLI flags (#10697) a41b134 Use the tracker retention period as the maximum expiry time. [KVL-1009] (#10700) e750ba5 Make warning less scary. (#10699) 5f120bd rotate release duty after 1.17.0-snapshot.20210824.7647.0.640fb683 (#10660) a17253f DPP-535 Verify postgres version (#10577) 301ce53 participant-integration-api: Add tests for ApiCommandService. [KVL-1009] (#10689) bd01a21 [DPP-418] Protect Participant TLS keys (#10629) 7ee1324 update NOTICES file (#10695) 7c392f3 update NOTICES file (#10693) 3db654e update NOTICES file (#10690) eff09c0 ledger-api-client: Wrap command submissions in a new class. [KVL-1009] (#10683) d54adb2 Ledger-API: Conformance tests for contract IDs suffixing (#10654) aa2e869 [Divulgence pruning] Pass divulged contract arguments through kvutils Write/ReadService [DPP-535] (#10598) 1a78313 Disable DeeplyNestedValueIT suite against canton in Daml repo (#10686) b5f9be3 participant-integration-api: Standardize tracker retention naming. (#10682) 2aa632e ledger-on-sql: Do not increment the dispatcher head on start. (#10684) eabb19d [ledger-api] Add deduplication_duration to deduplication period [kvl-1047] (#10676) 96ad9b5 [Divulgence pruning] All divulgence events pruning [DPP-483] (#10634) 0b7980d Update rules_haskell (#10674) 284edfc Fix FlywayMigrations datasource (#10666) adbe65f Document ActionFail vs CanAbort (#10657) 52e7a6d update compat versions for 1.17.0-snapshot.20210824.7647.0.640fb683 (#10667) f42e6b6 Expose pending contracts in triggers (#10672) 7cc6989 Add multiple ways of specifying deduplication [KVL-1047] (#10601) 53be19f participant-integration-api: Ensure that all waiting, failed, and closed trackers are cleaned up. (#10662) b27cde6 participant-integration-api: Move tracker code around, and tidy up tests. (#10663) fc9d359 Drop alternative rules from dlint config (#10646) 5204d3c Include committers in PartialTransaction root context (#10665) 387c68b Normalize transaction values within the engine (#10648) 430c1cc release 1.17.0-snapshot.20210824.7647.0.640fb683 (#10659) ef239fd participant-integration-api: Move `TrackerMap` code around. [KVL-1009] (#10653) ``` Changelog: ``` - [Sandbox] - Participant pruning is enabled in the sandbox-classic when the append-only schema is used - [Ledger Client Scala Bindings] A new variant of the LedgerClient class was added called `LedgerClientWithoutLedgerId`. This class does not need a ledger id at initialization. It was added to allow skipping any checks at initialization for use cases where either the ledger id is not known at initalization or no valid token can be fed at initialization for checking the ledger id. Furthermore for each classes `ActiveContractSetClient`, `CommandClient`, `PackageClient`, `TransactionClient`, `VersionClient` now exists a variant which doesn't depend on a ledger id at initialization and instead requires one for every function as parameter. Moreover the existing classes are extending these classes with overriding the methods and setting the default of the parameter with the given ledger id from initialization. The class `LedgerClientWithoutLedgerId` already makes usage of these variants e.g. `PackageClientWithoutLedgerId`. - [Ledger Client Scala Bindings] The function `transactionSource` of the class `LedgerClientBinding` now optionally accepts a token which is passed on to the unterlying call. - [JSON API] The Oracle database schema has changed; if using ``--query-store-jdbc-config``, you must rebuild the database by adding ``,start-mode=create-only``. See #10539. - [Trigger Service] ``--help`` no longer advertises unsupported JDBC options from JSON API. - [JSON API] [EE only] By default, on Oracle, sets up a JSON search index to speed up the queries endpoints. However, Oracle versions prior to 19.12 have an unrecoverably buggy implementation of this index; in addition, the current implementation fails on queries with strings >256 bytes, with no way to disable the index for that query. Pass the ``disableContractPayloadIndexing=true`` option as part of ``--query-store-jdbc-config`` to disable this index when creating the schema. See `issue #10539 <https://github.com/digital-asset/daml/pull/10539>`__. - [Integration Kit] Changes the Offset format to contain a version and therefore reduces the highest index size by one byte - [Ledger API Server] The command deduplication time is no longer used for determining the period of time to track the command before giving up. Instead, the gRPC deadline is used. If no deadline is provided (or if the deadline exceeds the command tracker retention period), the tracker retention period is used instead. - [Daml Repl] Fix a bug where bindings with out of scope types would result in error in following lines. - [Sandbox, participant] Added a flag to enable a new append-only database schema. This schema was designed to support significantly higher performance. In a future release, all applications will automatically migrate to the new schema. - [Ledger API Server] The command service now uses the tracker retention period (typically specified with the ``--tracker-retention-period`` command-line argument) as the maximum time to wait for a command to arrive on the completion stream. After this time, the command will time out, though it may still complete in the future. Previously, the deduplication period was used, but it was likely the tracker would be terminated before that anyway. The default tracker retention period is 5 minutes, unless otherwise specified. - [DPP-418] [Participant] Add support for supplying server's private key as an encrypted file and then decrypting it with the help of a secrets server. [Integration Kit] KV-based ledgers pass contract instances through the Write/ReadService, removing the need for backfilling divulged contract lookups. Note: KV Ledgers that have been created before this change will still be relying on backfilling lookups of divulged contracts, hence pruning of all divulged contracts may result in failing lookups for divulged contracts. ledger-api - add `deduplication_duration` as a future replacement for `deduplication_time` in the command proto definition ledger-api - Command deduplication period can now be specified by setting `deduplication_offset` instead of `deduplication_time` (only valid for v2 WriteService). This change is backwards compatible. ``` CHANGELOG_BEGIN CHANGELOG_END
aherrmann-da
pushed a commit
that referenced
this pull request
Sep 1, 2021
This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @aherrmann-da is in charge of this release. Commit log: ``` f058c2f DPP-368 clean up flags (#10711) 90ad24f [Divulgence pruning] Prune immediate divulgence [DPP-513] (#10691) 183934b Command dedup: add columns to completions in append-only schemas [KVL-1057] (#10652) 27c1333 LF: Drop outdated TODOs (#10725) 9be577a Enable pruning in the sandbox-classic when the append-only schema is used [DPP-567] (#10708) 9f072ae Ledger-API Conformance test for Contract ID V0 (#10717) 867547c DPP-368 enable append-only flag in sandbox (#10710) bdc511e [DPP-438] Change open-ended metric names into static ones (by removing partyName part) (#10706) 0c82006 [Divulgence pruning] Prune all divulged contracts only after migration offset [DPP-483] (#10661) 2555dbb Use soft references for values in the caches (#10715) 7fd5906 Add LedgerClientWithoutLedgerId next to the LedgerClient (#10681) 856c69c participant-integration-api: Increase a test timeout. (#10721) b86d07d remove non functioning oracle json array indices (#10720) f5e1756 sandbox-classic: Remove default parameters in `SqlLedger.Owner`. (#10718) 9ef3377 LF: Update specification with Contract ID Comparability check (#10703) e5c4734 enable JSON search index on Oracle by default (#10539) 1ded42f [DPP-418] Protect TLS keys - follow-up cleanup (#10696) 3fcd986 Introduce a new `Offset` format [KVL-1063] (#10668) a5781a6 update NOTICES file (#10714) 8985505 participant-integration-api: Use deadlines, not deduplication times, for expiring trackers. [KVL-1009] (#10704) 65025c2 sandbox-classic: Add ErrorInfo metadata for rejections. [KVL-1048] (#10707) 97bda3c LF: V1 Contract ID check in Preprocessor (#10687) c2f90ef Add CLI option to force disabling of participant deduplication (#10698) 6016633 Construct ParsedModule directly in Daml Repl (#10701) bbdf16a DPP-368 unhide append-only CLI flags (#10697) a41b134 Use the tracker retention period as the maximum expiry time. [KVL-1009] (#10700) e750ba5 Make warning less scary. (#10699) 5f120bd rotate release duty after 1.17.0-snapshot.20210824.7647.0.640fb683 (#10660) a17253f DPP-535 Verify postgres version (#10577) 301ce53 participant-integration-api: Add tests for ApiCommandService. [KVL-1009] (#10689) bd01a21 [DPP-418] Protect Participant TLS keys (#10629) 7ee1324 update NOTICES file (#10695) 7c392f3 update NOTICES file (#10693) 3db654e update NOTICES file (#10690) eff09c0 ledger-api-client: Wrap command submissions in a new class. [KVL-1009] (#10683) d54adb2 Ledger-API: Conformance tests for contract IDs suffixing (#10654) aa2e869 [Divulgence pruning] Pass divulged contract arguments through kvutils Write/ReadService [DPP-535] (#10598) 1a78313 Disable DeeplyNestedValueIT suite against canton in Daml repo (#10686) b5f9be3 participant-integration-api: Standardize tracker retention naming. (#10682) 2aa632e ledger-on-sql: Do not increment the dispatcher head on start. (#10684) eabb19d [ledger-api] Add deduplication_duration to deduplication period [kvl-1047] (#10676) 96ad9b5 [Divulgence pruning] All divulgence events pruning [DPP-483] (#10634) 0b7980d Update rules_haskell (#10674) 284edfc Fix FlywayMigrations datasource (#10666) adbe65f Document ActionFail vs CanAbort (#10657) 52e7a6d update compat versions for 1.17.0-snapshot.20210824.7647.0.640fb683 (#10667) f42e6b6 Expose pending contracts in triggers (#10672) 7cc6989 Add multiple ways of specifying deduplication [KVL-1047] (#10601) 53be19f participant-integration-api: Ensure that all waiting, failed, and closed trackers are cleaned up. (#10662) b27cde6 participant-integration-api: Move tracker code around, and tidy up tests. (#10663) fc9d359 Drop alternative rules from dlint config (#10646) 5204d3c Include committers in PartialTransaction root context (#10665) 387c68b Normalize transaction values within the engine (#10648) 430c1cc release 1.17.0-snapshot.20210824.7647.0.640fb683 (#10659) ef239fd participant-integration-api: Move `TrackerMap` code around. [KVL-1009] (#10653) ``` Changelog: ``` - [Sandbox] - Participant pruning is enabled in the sandbox-classic when the append-only schema is used - [Ledger Client Scala Bindings] A new variant of the LedgerClient class was added called `LedgerClientWithoutLedgerId`. This class does not need a ledger id at initialization. It was added to allow skipping any checks at initialization for use cases where either the ledger id is not known at initalization or no valid token can be fed at initialization for checking the ledger id. Furthermore for each classes `ActiveContractSetClient`, `CommandClient`, `PackageClient`, `TransactionClient`, `VersionClient` now exists a variant which doesn't depend on a ledger id at initialization and instead requires one for every function as parameter. Moreover the existing classes are extending these classes with overriding the methods and setting the default of the parameter with the given ledger id from initialization. The class `LedgerClientWithoutLedgerId` already makes usage of these variants e.g. `PackageClientWithoutLedgerId`. - [Ledger Client Scala Bindings] The function `transactionSource` of the class `LedgerClientBinding` now optionally accepts a token which is passed on to the unterlying call. - [JSON API] The Oracle database schema has changed; if using ``--query-store-jdbc-config``, you must rebuild the database by adding ``,start-mode=create-only``. See #10539. - [Trigger Service] ``--help`` no longer advertises unsupported JDBC options from JSON API. - [JSON API] [EE only] By default, on Oracle, sets up a JSON search index to speed up the queries endpoints. However, Oracle versions prior to 19.12 have an unrecoverably buggy implementation of this index; in addition, the current implementation fails on queries with strings >256 bytes, with no way to disable the index for that query. Pass the ``disableContractPayloadIndexing=true`` option as part of ``--query-store-jdbc-config`` to disable this index when creating the schema. See `issue #10539 <https://github.com/digital-asset/daml/pull/10539>`__. - [Integration Kit] Changes the Offset format to contain a version and therefore reduces the highest index size by one byte - [Ledger API Server] The command deduplication time is no longer used for determining the period of time to track the command before giving up. Instead, the gRPC deadline is used. If no deadline is provided (or if the deadline exceeds the command tracker retention period), the tracker retention period is used instead. - [Daml Repl] Fix a bug where bindings with out of scope types would result in error in following lines. - [Sandbox, participant] Added a flag to enable a new append-only database schema. This schema was designed to support significantly higher performance. In a future release, all applications will automatically migrate to the new schema. - [Ledger API Server] The command service now uses the tracker retention period (typically specified with the ``--tracker-retention-period`` command-line argument) as the maximum time to wait for a command to arrive on the completion stream. After this time, the command will time out, though it may still complete in the future. Previously, the deduplication period was used, but it was likely the tracker would be terminated before that anyway. The default tracker retention period is 5 minutes, unless otherwise specified. - [DPP-418] [Participant] Add support for supplying server's private key as an encrypted file and then decrypting it with the help of a secrets server. [Integration Kit] KV-based ledgers pass contract instances through the Write/ReadService, removing the need for backfilling divulged contract lookups. Note: KV Ledgers that have been created before this change will still be relying on backfilling lookups of divulged contracts, hence pruning of all divulged contracts may result in failing lookups for divulged contracts. ledger-api - add `deduplication_duration` as a future replacement for `deduplication_time` in the command proto definition ledger-api - Command deduplication period can now be specified by setting `deduplication_offset` instead of `deduplication_time` (only valid for v2 WriteService). This change is backwards compatible. ``` CHANGELOG_BEGIN CHANGELOG_END Co-authored-by: Azure Pipelines DAML Build <[email protected]>
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.
Main change
sandbox-classicwhen the append-only schema is usedPull 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.