Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce filter table [DPP-714] #11687

Merged
merged 1 commit into from
Nov 14, 2021
Merged

Introduce filter table [DPP-714] #11687

merged 1 commit into from
Nov 14, 2021

Conversation

nmarton-da
Copy link
Contributor

  • Adding new table, indexes
  • Adding data migration (PostgreSQL only)
  • Removing unnecessary indexes
  • Adding DbDto, Schema integration for the filter table
  • Adding generation of CreateFilter Dto-s to UpdateToDbDto
  • Adding sequential ID generation to sandbox-classic and parallel-ingestion
  • Adding StorageBackend support for pruning, reset
  • Adding EventStorageBackend support for two phase retrival queries
  • Extending/adapting unit tests
  • Adding PostgreSQL VACUUM ANALYZE Flyway migration script as well (since this is the last persistence change in the ACS feature track)

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
  • If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

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.

* Adding new table, indexes
* Adding data migration (PostgreSQL only)
* Removing unnecessary indexes
* Adding DbDto, Schema integration for the filter table
* Adding generation of CreateFilter Dto-s to UpdateToDbDto
* Adding sequential ID generation to sandbox-classic and parallel-ingestion
* Adding StorageBackend support for pruning, reset
* Adding EventStorageBackend support for two phase retrival queries
* Extending/adapting unit tests
* Adding PostgreSQL VACUUM ANALYZE Flyway migration script as well (since this is the last persistence change in the ACS feature track)

changelog_begin
changelog_end
@nmarton-da nmarton-da requested a review from a team as a code owner November 13, 2021 22:25

CREATE INDEX idx_participant_events_create_filter_party_template_seq_id_idx ON participant_events_create_filter(party_id, template_id, event_sequential_id);
CREATE INDEX idx_participant_events_create_filter_party_seq_id_idx ON participant_events_create_filter(party_id, event_sequential_id);
CREATE INDEX idx_participant_events_create_seq_id_idx ON participant_events_create_filter(event_sequential_id);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meiersi-da it's a pity: but we need this in order to efficiently cleanup the over-spill entries for parallel ingestion. Do you know maybne a way to somehow "reuse" the above indexes for this purpose? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was always expecting to required the index on event_sequential_id. It's required for pruning anyways isn't it?

-- Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
-- SPDX-License-Identifier: Apache-2.0

VACUUM ANALYZE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the last step in the chain of migrations that implement the new ACS queries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly

@@ -30,6 +30,7 @@ private[backend] trait StorageBackendTestsInitializeIngestion
dtoPackageEntry(offset(3)),
// 4: transaction with create node
dtoCreate(offset(4), 1L, "#4"),
DbDto.CreateFilter(1L, someTemplateId.toString, someParty.toString),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DbDto.CreateFilter(1L, someTemplateId.toString, someParty.toString),
dtoCreateFilter(1L, someParty),

we could add a helper like for all other DTOs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure why not

@@ -195,6 +195,8 @@ class ParallelIndexerSubscriptionSpec extends AnyFlatSpec with Matchers {
someEventDivulgence,
someParty,
someEventCreated,
DbDto.CreateFilter(0L, "", ""),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DbDto.CreateFilter(0L, "", ""),
someCreateFilter,

for consistency

Comment on lines 50 to +51
dtoCreate(offset(9), 4L, "#9"),
DbDto.CreateFilter(4L, someTemplateId.toString, someParty.toString),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many places where we ingest a create event without a corresponding filter event. The same already happens with completion events (not every dtoCreate is followed by a dtoCompletion) - the missing DTOs are not relevant for the respective test.
I'm still slightly worried about the fact that we have an increasing number of DTO sequences that cannot be produced by a real indexer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of these tests is not to test the real indexer, rather some targeted functionality. These are unit tests.

@nmarton-da nmarton-da merged commit fb840fa into main Nov 14, 2021
@nmarton-da nmarton-da deleted the dpp-714-filter-table branch November 14, 2021 22:12
azure-pipelines bot pushed a commit that referenced this pull request Nov 17, 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.

@victormueller-da is in charge of this release.

Commit log:
```
ccbf714 expand the text about errors in the log [DPP-755] (#11723)
fbc436d [DPP-739][Self-service error codes] Adapt QueueBackedTracker error codes (#11719)
07cff7c [DPP-417][Self-service error-codes] Document error categories.  (#11727)
c866fa6 Optimize data migrations for performance [DPP-714] (#11714)
f801314 Drop exercise with actor from LF spec (#11721)
a63e091 kvutils: Use consistent argument name casing in KVErrors (#11609)
c1f4beb Ledger ID mismatch does not change error codes. (#11720)
b5b29fd test_suite for http-json, http-json-oracle integration tests (#11713)
a29fee9 [Self-service error-codes] Add introductory section to self-service error codes page. (#11712)
639c431 libs-scala: Move `SourceQueueResourceOwner` from the Integration Kit (#11708)
05f604d [Self-service error codes] Enrich migration guide [DPP-755] (#11711)
2d5b548 Use explicit party specifications in Daml Script over JSON API (#11678)
844e5d0 participant-integration-api: Propagate log context through the indexer. (#11710)
c969266 Formatting feedback from: Log execution context throwables as error (#11707)
ec6d7cc [Self-service error codes] Error code definitions revisiting [DPP-675] (#11686)
a1fc5c6 Simplify handling for EqualList builtin. (#11676)
90ad968 Workflow for the ledger-api-bench-tool  defined in YAML file [DPP-669] (#11682)
1791a52 [TS-Bindings] Changes to re-enable ws multiplexing (#11681)
fd94099 update compat versions for 1.18.0-snapshot.20211111.8349.0.d938a44c (#11667)
612a838 ledger-api-bench-tool: Allocate observers indexed from 0 until the total number (#11700)
41d2d95 Factorize Java codegen integration test build defs (#11704)
bf86ee4 Log execution context throwables as error (#11702)
69471d6 ledger-api-client: Fix a log message related to an empty submission ID (#11695)
775b41c LF: Reintroduce TransactionVersion.asVersionedTransaction (#11692)
8015f60 Add `cpu:2` tag to extractor tests (#11697)
8e08450 [Self-service error codes] No redundant logging in Ledger API (#11649)
18433eb fix arity of TArrow type constructor in Daml-LF spec (#11677)
fb840fa Introduce filter table [DPP-714] (#11687)
cb5a675 Activate template_id interning [DPP-713] (#11680)
c4da8e3 Limit gRPC wire transport size legacy error codes size (#11666)
72f3dd4 Activate party interning [DPP-712] (#11663)
57f1b86 Improve byInterfaceNodes based on suggestions (#11672)
d6ca68d [Self-service error codes] Adapt Sandbox classic rejection error codes (#11622)
15b925a LF: Compute transaction version as the maximum of the children (#11626)
ebdab31 rotate release duty after 1.18.0-snapshot.20211109.8328.0.92181161 (#11624)
7eb22dd LF: drop forgotten deprecate Node aliases. (#11673)
b0463aa Move script auth tests to postgresql (#11670)
8c46559 Remove unnecessary constructors: SEDamlException, SEImportValue, from SExpr{0,1} (#11668)
c109552 Disable ofInterestRule by default (#11637)
409864f Use by_interface field when fetching transaction dependencies. (#11662)
81e2450 [DDP-596][Self-service error codes] Add migration guide (#11632)
3192d5e Different types before after closure conversion (#11661)
2f4476c [Self-service error codes] Fill in missing places with submission id for correlation id (#11593)
d3bb036 Fix sandbox database config in script & trigger tests (#11655)
22d916b party-set arguments for JSON API (#11454)
3d109b6 Release snapshot 1.18.0-snapshot.20211111.8349.0.d938a44c (#11664)
```
Changelog:
```

- [Daml Script] When run over the JSON API, Daml Script can now use a
  subset of the claims in the token. E.g., `submit p` works even if you
  have a token with `actAs = [p, p2]`.

- [Integration Kit] - ledger-api-bench-tool - All stream configuration parameters can be defined through the YAML configuration file.
[TS-BINDINGS] Re-enable ws multiplexing for stream queries after resolving the reconnect connection close bug associated with ws state and liveness.
- [JSON API] ``actAs`` and ``readAs`` may be specified for create, exercise,
  create-and-exercise, non-WS fetch, and non-WS query.
  See `issue #11454 <https://github.com/digital-asset/daml/pull/11454>`__.
```

CHANGELOG_BEGIN
CHANGELOG_END
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants