[Divulgence pruning] Prune immediate divulgence [DPP-513] #10691

tudor-da · 2021-08-27T09:02:43Z

[Divulgence pruning] Prune immediate divulgence

Adapt CommonStorageBackend.pruneEvents to prune all immediately divulged events
Adapt ParticipantPruningIT to assert immediate divulgence pruning
Adapt ParticipantPruningIT tests for divulgence pruning to assert the ACS before and after pruning

CHANGELOG_BEGIN
CHANGELOG_END

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

tudor-da · 2021-08-31T12:34:15Z

ledger/ledger-on-memory/BUILD.bazel

@@ -226,7 +226,7 @@ conformance_test(
        "--verbose",
        "--include=ParticipantPruningIT",
        # Disable tests targeting only append-only schema functionality
-        "--exclude=ParticipantPruningIT:PRLocalAndNonLocalRetroactiveDivulgences,ParticipantPruningIT:PRRetroactiveDivulgences",
+        "--exclude=ParticipantPruningIT:PRLocalAndNonLocalRetroactiveDivulgences,ParticipantPruningIT:PRRetroactiveDivulgences,ParticipantPruningIT:PRDisclosureAndRetroactiveDivulgence",


Normalization of these conformance test will be done in a subsequent PR. For clarity/conciseness, in this PR, I blacklist the tests that cannot run on single-participant setups.

meiersi-da

👍 LGTM

ledger/ledger-api-test-tool-on-canton/BUILD.bazel

...pant-integration-api/src/main/scala/platform/store/backend/common/CommonStorageBackend.scala

tudor-da · 2021-08-31T15:56:43Z

The failures are due to updated master which is conflicting with the changes in here.. weirdly enough, no merge conflicts 🤔

tudor-da · 2021-08-31T15:59:00Z

The failures are due to updated master which is conflicting with the changes in here.. weirdly enough, no merge conflicts 🤔

Rebased

nmarton-da

minor comments, good stuff

nmarton-da · 2021-08-31T17:02:57Z

...pant-integration-api/src/main/scala/platform/store/backend/common/CommonStorageBackend.scala

@@ -524,6 +525,32 @@ private[backend] trait CommonStorageBackend[DB_BATCH] extends StorageBackend[DB_
            )"""


This Event pruning would be probably more well suited in the EventStorageBackendTemplate, especiallly since it using templates now. (if you are under time pressure never mind, we can do it in the cleanup epic)

That's a good point. I'll add a TODO to it and please let's leave it for the cleanup epic.

nmarton-da · 2021-08-31T17:11:44Z

...ntegration-api/src/main/scala/platform/store/backend/postgresql/PostgresStorageBackend.scala

+    override def arrayContains(arrayColumnName: String, elementColumnName: String): String =
+      s"$elementColumnName = any($arrayColumnName)"
+
+    override def isTrue(booleanColumnName: String): String = booleanColumnName


how about defining this in QueryStrategy itself and overriding only in Oracle?

I'm bearish on overriding, in general, and here, for two lines shaved, I would keep them clear. Traversing inheritance levels for understanding functionality is not my piece of cake. I'd change it if you think this degrades coherence in the implementation, though

Wouldn't we have to override in both Oracle and H2 when defining in QueryStrategy?

Oh, you were probably talking only about isTrue() @nmarton-da . Sorry, I thought about both isTrue() and arrayContains().

@nmarton-da I'll merge this as it is. If we'll reach a different conclusion on this topic, I'll amend it in a separate PR.

nmarton-da · 2021-08-31T17:15:04Z

...pant-integration-api/src/main/scala/platform/store/backend/common/CommonStorageBackend.scala

+            and not exists (
+              select 1
+              from party_entries p
+              where p.typ = 'accept'


was this the issue with the string comparison? if yes, would be nice to add a short comment that without this some fields got empty which lead to oracle issues etcetcetc

If you are referring to the Oracle-specific blocker that I've mentioned offline, it was the comparison participant_all_divulged_contracts_pruned_up_to_inclusive (which was nullable/empty-stringed) to the event_offset. And in Oracle '' does not compare lexicogrphically as expected.

Which makes me wonder: this is a very common comparison that we're doing. howcome it did not pop out in other places 🤔 ?

* Adapt CommonStorageBackend.pruneEvents to prune all immediately divulged events * Adapt ParticipantPruningIT to assert immediate divulgence pruning * Adapt ParticipantPruningIT tests for divulgence pruning to assert the ACS before and after pruning CHANGELOG_BEGIN CHANGELOG_END

@aherrmann-da

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

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]>

tudor-da force-pushed the tudor/dpp-513-prune-immediate-divulgence branch 4 times, most recently from a87dc37 to 02c7db5 Compare August 31, 2021 12:21

tudor-da changed the title ~~WIP~~ [Divulgence pruning] Prune disclosed contracts [DPP-513] Aug 31, 2021

tudor-da changed the title ~~[Divulgence pruning] Prune disclosed contracts [DPP-513]~~ [Divulgence pruning] Prune immediate divulgence [DPP-513] Aug 31, 2021

tudor-da marked this pull request as ready for review August 31, 2021 12:32

tudor-da requested review from a team as code owners August 31, 2021 12:32

tudor-da commented Aug 31, 2021

View reviewed changes

meiersi-da reviewed Aug 31, 2021

View reviewed changes

ledger/ledger-api-test-tool-on-canton/BUILD.bazel Outdated Show resolved Hide resolved

...pant-integration-api/src/main/scala/platform/store/backend/common/CommonStorageBackend.scala Show resolved Hide resolved

tudor-da force-pushed the tudor/dpp-513-prune-immediate-divulgence branch from 355dadd to 465f8b4 Compare August 31, 2021 15:58

nmarton-da approved these changes Aug 31, 2021

View reviewed changes

tudor-da force-pushed the tudor/dpp-513-prune-immediate-divulgence branch from 465f8b4 to bbe45e6 Compare August 31, 2021 18:00

kamil-da approved these changes Aug 31, 2021

View reviewed changes

tudor-da force-pushed the tudor/dpp-513-prune-immediate-divulgence branch from bbe45e6 to 1a981c2 Compare August 31, 2021 18:48

tudor-da added the automerge label Aug 31, 2021

mergify bot merged commit 90ad24f into main Aug 31, 2021

mergify bot deleted the tudor/dpp-513-prune-immediate-divulgence branch August 31, 2021 20:19

tudor-da added 5 commits August 31, 2021 20:47

Adapt conformance tests

d9aae2c

Adapt disclosure query for Oracle

47f73f8

Addressed review comments

aec2a5c

Rebased

1a981c2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Divulgence pruning] Prune immediate divulgence [DPP-513] #10691

[Divulgence pruning] Prune immediate divulgence [DPP-513] #10691

tudor-da commented Aug 27, 2021 •

edited

Loading

tudor-da Aug 31, 2021

meiersi-da left a comment

tudor-da commented Aug 31, 2021

tudor-da commented Aug 31, 2021

nmarton-da left a comment

nmarton-da Aug 31, 2021

tudor-da Aug 31, 2021

nmarton-da Aug 31, 2021

tudor-da Aug 31, 2021 •

edited

Loading

kamil-da Aug 31, 2021

kamil-da Aug 31, 2021

tudor-da Aug 31, 2021 •

edited

Loading

nmarton-da Aug 31, 2021

tudor-da Aug 31, 2021

tudor-da Aug 31, 2021 •

edited

Loading

		@@ -524,6 +525,32 @@ private[backend] trait CommonStorageBackend[DB_BATCH] extends StorageBackend[DB_
		)"""

[Divulgence pruning] Prune immediate divulgence [DPP-513] #10691

[Divulgence pruning] Prune immediate divulgence [DPP-513] #10691

Conversation

tudor-da commented Aug 27, 2021 • edited Loading

Pull Request Checklist

Choose a reason for hiding this comment

meiersi-da left a comment

Choose a reason for hiding this comment

tudor-da commented Aug 31, 2021

tudor-da commented Aug 31, 2021

nmarton-da left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tudor-da Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tudor-da Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tudor-da Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

tudor-da commented Aug 27, 2021 •

edited

Loading

tudor-da Aug 31, 2021 •

edited

Loading

tudor-da Aug 31, 2021 •

edited

Loading

tudor-da Aug 31, 2021 •

edited

Loading