Command dedup: add columns to completions in append-only schemas [KVL-1057] #10652

fabiotudone-da · 2021-08-24T09:37:52Z

Description

This is a further change in a series (after #10619) to let the Ledger API provide more command de-duplication information in completions as well (only the submission rank is deferred to later stages), so that the application can use it in correlation with the one provided at command submission time.

It updates the index DB append-only schema to allow storing the additional command de-duplication information and provides the read/write logic for it.

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.

CHANGELOG_BEGIN CHANGELOG_END

…letions/proto-additions

…etions

CHANGELOG_BEGIN CHANGELOG_END

fabiotudone-da · 2021-08-24T09:42:06Z

@digital-asset/kv-participant Could you please have a first stab at the schema changes?

…fo-to-completions

...in/resources/db/migration/postgres-appendonly/V108_add_deduplication_info_to_completions.sql

...gration-api/src/main/resources/db/migration/h2database-appendonly/V1__Append_only_schema.sql

...in/resources/db/migration/postgres-appendonly/V108_add_deduplication_info_to_completions.sql

fabiotudone-da

A couple of additional things I am not sure about:

Do we have a policy for the position of newly added columns?
I couldn't find another instance of duration, is the double i64+i32 column (as in protobuf) OK?

nmarton-da · 2021-08-24T11:47:46Z

A couple of additional things I am not sure about:
* Do we have a policy for the position of newly added columns?

AFAIK no policy regarding position ATM

* I couldn't find another instance of the duration, is the double i64+i32 column (as in protobuf) OK?

Do you need such precision? Wouldn't be seconds or millis enough (resulting in one bigint column)?
Besides no idea what datatype best to store duration, I would go with millis/bigint

nmarton-da · 2021-08-24T11:52:11Z

General comment to this PR
Not sure about you intentions, but it would be great (most probably necessary) to make the DB changes together with the DB facing code changes:

Reviewers would gain more clarity where the schema is heading semantically
It would be clear how backwards compatibility is ensured
It would probably also include db facing tests, which would ensure smooth operation between schema and code
It would successfully prevent possible multiple iterations on the schema due to integration with code (indexes, data-type changes, constraint changes, migration changes etc)

fabiotudone-da · 2021-08-24T12:42:12Z

General comment to this PR

@nmarton-da Thanks for the feedback; the intention was to break down the work in small, separate reviews as much as possible, starting with the ones requiring the most feedback (like the ledger API proto and the DB schema, the latter purely on a technical level initially).
I think creating additional PRs on top of this one with the code changes (and waiting for a review on them too before merging anything) could have the advantages of both approaches. WDYT?

fabiotudone-da · 2021-08-24T12:57:26Z

Do you need such precision? Wouldn't be seconds or millis enough (resulting in one bigint column)?
Besides no idea what datatype best to store duration, I would go with millis/bigint

Good point, I couldn't find any mention of resolution restrictions w.r.t. the API type (i.e., protobuf's). @SamirTalwar-DA Do you know anything more here?

nmarton-da · 2021-08-24T13:08:49Z

General comment to this PR

@nmarton-da Thanks for the feedback; the intention was to break down the work in small, separate reviews as much as possible, starting with the ones requiring the most feedback (like the ledger API proto and the DB schema, the latter purely on a technical level initially).
I think creating additional PRs on top of this one with the code changes (and waiting for a review on them too before merging anything) could have the advantages of both approaches. WDYT?

I think the schema change and the code which works with that should be same unit in terms of work / mergable / PR.
How about keeping this a draft, after agreement reached, closing it, and then open another one which is about the change of the persistence with this content, and with the code changes?

Don't get me wrong please, small PRs is a very good thing, but there must be a PR which is too small :) I claim making db changes without the supporting code falls into the too small category. Also don't get me wrong please on the other end neither: I am not saying that because that should belong together with something else, then everything needs to belong together: feel free to maintain granular changes later as you see fit!

TLDR:
I would applaud if we could have a selfcontained PR:

having db schema changes
having StorageBackend code changes
having JdbcLedgerDao code changes if needed
and db facing tests proving successful SQL-scala integration
...together in one PR

…fo-to-completions

…as, as in PSQL

rautenrieth-da · 2021-08-30T10:07:13Z

ledger/participant-integration-api/src/main/scala/platform/store/backend/UpdateToDbDto.scala

+    val (deduplicationTimeSeconds, deduplicationTimeNanos) =
+      completionInfo.optDeduplicationPeriod
+        .flatMap {


Suggested change

val (deduplicationTimeSeconds, deduplicationTimeNanos) =

completionInfo.optDeduplicationPeriod

.flatMap {

val (deduplicationTimeSeconds, deduplicationTimeNanos, deduplicationOffset) =

completionInfo.optDeduplicationPeriod

.flatMap {

You are also calling completionInfo.optDeduplicationPeriod.flatMap below, you could move it up here to have a single place to handle all three variants.

482891e and 804c99e

...ant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestValues.scala

rautenrieth-da · 2021-08-30T13:06:18Z

...ant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestValues.scala

@@ -41,6 +41,7 @@ private[backend] object StorageBackendTestValues {
    ParameterStorageBackend.IdentityParams(someLedgerId, someParticipantId)
  val someParty: Ref.Party = Ref.Party.assertFromString("party")
  val someApplicationId: Ref.ApplicationId = Ref.ApplicationId.assertFromString("application_id")
+  val someSubmissionId: String = "submission_id"


Can you make this a Ref.SubmissionId? All other some* values are properly typed as well.

rautenrieth-da · 2021-08-30T13:17:42Z

...ticipant-integration-api/src/test/suite/scala/platform/store/backend/UpdateToDbDtoSpec.scala

-      val dtos = UpdateToDbDto(someParticipantId, valueSerialization, compressionStrategy)(
-        someOffset
-      )(update).toList
+      forAll(deduplicationPeriods) {


Maybe you could have left "handle TransactionAccepted (single create node)" alone and added a new test "handle TransactionAccepted (all deduplication data)". This would make it more clear what the purpose of each test is, at the cost of duplicated code.

rautenrieth-da · 2021-08-30T13:19:04Z

...er/participant-integration-api/src/main/scala/platform/store/CompletionFromTransaction.scala

+      maybeDeduplicationTimeNanos: Option[Int],
+      maybeDeduplicationTimeSeconds: Option[Long],
+  ): Completion.DeduplicationPeriod =
+    // The only invariant tha should hold, considering legacy data, is that either


Suggested change

// The only invariant tha should hold, considering legacy data, is that either

// The only invariant that should hold, considering legacy data, is that either

…torageBackendTemplate calls

nmarton-da

Looks solid.
Thanks for your patience regarding the back and forth-s

nmarton-da · 2021-08-31T08:51:16Z

...er/participant-integration-api/src/main/scala/platform/store/CompletionFromTransaction.scala

@@ -60,4 +81,82 @@ private[platform] object CompletionFromTransaction {
      recordTime = Some(fromInstant(recordTime)),
      offset = Some(LedgerOffset.of(LedgerOffset.Value.Absolute(offset.toApiString))),
    )
+
+  private def toApiCompletion(


quite some branches here, how about adding some unit tests?

It's mostly because of scalapb plus proto's "non-optional optionals"; I simplified it further in 2749c75 and I think a unit test at this level doesn't add much value because it's very thin logic and the test would basically duplicate it in order to check it. I think testing the CompletionFromTransaction unit at its interface (i.e., acceptedCompletion and rejectedCompletion) is in a similar situation as it just delegates to the private toXXX utilities.

Also, this logic should already have full coverage by the DB-level integration test cases in StorageBackendTestsCompletions. WDYT?

I'll merge this as soon as the build passes and we'll deal with further improvements in a separate PR, if need be.

nmarton-da · 2021-08-31T08:54:59Z

ledger/participant-integration-api/src/main/scala/platform/store/backend/UpdateToDbDto.scala

+          case DeduplicationDuration(duration) =>
+            (None, Some(duration.getSeconds), Some(duration.getNano))
+        } match {
+        case Some(value) => value


how about .getOrElse((None, None, None)) instead?

…fo-to-completions

@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 <support@digitalasset.com>

fabiotudone-da added 10 commits August 19, 2021 12:31

Augment completion.proto with deduplication-related info

8dfedb4

CHANGELOG_BEGIN CHANGELOG_END

Merge branch 'main' into fabiotudone-da/command-dedup/ledger-api-comp…

9768373

…letions/proto-additions

Explicitly specify fields not yet filled in when building Completion

e43f2fc

Time-based deduplication periods are measured in record time of compl…

d566b97

…etions

Add deduplication_offset as a deduplication_period option

89336ab

Don't skip proto field numbers

eb14dd9

CompletionFromTransaction: use default Completion constructor

1ba2045

submission_rank: reserve proto field for future use

efa01ff

Add comment about reserved proto field

73fd7a2

Add command deduplication columns to completions in append-only schemas

8e54bb5

CHANGELOG_BEGIN CHANGELOG_END

fabiotudone-da requested a review from a team August 24, 2021 09:42

Base automatically changed from fabiotudone-da/command-dedup/ledger-api-completions/proto-additions to main August 24, 2021 10:55

Merge branch 'main' into fabiotudone-da/command-dedup/index-db-add-in…

35c6ded

…fo-to-completions

fabiotudone-da commented Aug 24, 2021

View reviewed changes

...in/resources/db/migration/postgres-appendonly/V108_add_deduplication_info_to_completions.sql Outdated Show resolved Hide resolved

fabiotudone-da requested review from a user, nicu-da and miklos-da August 24, 2021 11:01

ghost reviewed Aug 24, 2021

View reviewed changes

fabiotudone-da added 3 commits August 24, 2021 13:32

Remove duplicated application_id field from Postgres and H2 schemas

8079673

Make application_id and submission_id nullable

3e5fa10

Re-generate migrations' sha256

f68a130

fabiotudone-da commented Aug 24, 2021

View reviewed changes

Merge branch 'main' into fabiotudone-da/command-dedup/index-db-add-in…

abf65e3

…fo-to-completions

fabiotudone-da marked this pull request as draft August 30, 2021 11:44

fabiotudone-da added 4 commits August 30, 2021 13:59

Fix completions' application_id: it must be non-nullable in all schem…

24d1d6e

…as, as in PSQL

Actually use newly read columns

eb44d3c

Add DB round-trip tests for new fields in StorageBackendTestValues

5643b95

Format

51ab254

rautenrieth-da reviewed Aug 30, 2021

View reviewed changes

fabiotudone-da added 10 commits August 30, 2021 15:36

StorageBackendTestValues: property type someSubmissionId

87da1dd

UpdateToDbDto: remove duplicated logic

482891e

Simplify UpdateToDbDto.commandCompletion

804c99e

Fix comment in CompletionFromTransaction.toApiDeduplicationPeriod

f32346e

Fix CompletionFromTransaction.toApiCompletion

4ebd1e0

Fix deduplication_offset in Schema

b1bc3d4

Make parameters explicit in CompletionFromTransaction and CompletionS…

85ed113

…torageBackendTemplate calls

Shorten error message in CompletionFromTransaction

8da5af0

Fix StorageBackendTestsCompletions

166cd83

Fix CompletionFromTransaction

3b7a9e5

fabiotudone-da marked this pull request as ready for review August 30, 2021 20:45

fabiotudone-da requested review from nmarton-da and rautenrieth-da August 30, 2021 20:45

Separate command dedup-related test cases in UpdateToDbDtoSpec

bde3d1e

nmarton-da approved these changes Aug 31, 2021

View reviewed changes

fabiotudone-da added 4 commits August 31, 2021 13:16

Simplify further UpdateToDbDto.commandCompletion

4a94562

Simplify CompletionFromTransaction.toApiCompletion

2749c75

Format

db74dd0

Merge branch 'main' into fabiotudone-da/command-dedup/index-db-add-in…

9c305fc

…fo-to-completions

fabiotudone-da enabled auto-merge (squash) August 31, 2021 18:22

fabiotudone-da merged commit 183934b into main Aug 31, 2021

fabiotudone-da deleted the fabiotudone-da/command-dedup/index-db-add-info-to-completions branch August 31, 2021 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Command dedup: add columns to completions in append-only schemas [KVL-1057] #10652

Command dedup: add columns to completions in append-only schemas [KVL-1057] #10652

fabiotudone-da commented Aug 24, 2021 •

edited

Loading

fabiotudone-da commented Aug 24, 2021

fabiotudone-da left a comment •

edited

Loading

nmarton-da commented Aug 24, 2021 •

edited

Loading

nmarton-da commented Aug 24, 2021

fabiotudone-da commented Aug 24, 2021

fabiotudone-da commented Aug 24, 2021 •

edited

Loading

nmarton-da commented Aug 24, 2021

rautenrieth-da Aug 30, 2021

fabiotudone-da Aug 30, 2021

rautenrieth-da Aug 30, 2021

fabiotudone-da Aug 30, 2021

rautenrieth-da Aug 30, 2021

rautenrieth-da Aug 30, 2021

fabiotudone-da Aug 30, 2021

nmarton-da left a comment

nmarton-da Aug 31, 2021

fabiotudone-da Aug 31, 2021

fabiotudone-da Aug 31, 2021

nmarton-da Aug 31, 2021

fabiotudone-da Aug 31, 2021

	// The only invariant tha should hold, considering legacy data, is that either
	// The only invariant that should hold, considering legacy data, is that either

Command dedup: add columns to completions in append-only schemas [KVL-1057] #10652

Command dedup: add columns to completions in append-only schemas [KVL-1057] #10652

Conversation

fabiotudone-da commented Aug 24, 2021 • edited Loading

Description

Pull Request Checklist

fabiotudone-da commented Aug 24, 2021

fabiotudone-da left a comment • edited Loading

Choose a reason for hiding this comment

nmarton-da commented Aug 24, 2021 • edited Loading

nmarton-da commented Aug 24, 2021

fabiotudone-da commented Aug 24, 2021

fabiotudone-da commented Aug 24, 2021 • edited Loading

nmarton-da commented Aug 24, 2021

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiotudone-da commented Aug 24, 2021 •

edited

Loading

fabiotudone-da left a comment •

edited

Loading

nmarton-da commented Aug 24, 2021 •

edited

Loading

fabiotudone-da commented Aug 24, 2021 •

edited

Loading