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.

Sign up for GitHub

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

Jump to bottom

Nest stakeholders in contracts table as JSON arrays #9484

Merged
merged 5 commits into from
Apr 27, 2021
Merged

Nest stakeholders in contracts table as JSON arrays #9484

merged 5 commits into from
Apr 27, 2021

Conversation

stefanobaghino-da
Copy link
Contributor

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.

@stefanobaghino-da
Copy link
Contributor Author

There seems to be an error with a SELECT clause that raises some 1011 "Invalid column index" error, but I cannot spot it.

@stefanobaghino-da stefanobaghino-da marked this pull request as ready for review April 26, 2021 10:10
@stefanobaghino-da
Copy link
Contributor Author

There seems to be an error with a SELECT clause that raises some 1011 "Invalid column index" error, but I cannot spot it.

Ultimately I believe the issue was a very well hidden missing parentheses that unfortunately caused this very misleading error to appear. Now all tests pass locally.

@S11001001
Copy link
Contributor

b285fae with #9492 (no extra indices):

---- SyncQueryRequest ----------------------------------------------------------
> Number of requests                                   500 (OK=500    KO=-     )
> Min. response time                                    22 (OK=22     KO=-     )
> Max. response time                                  1588 (OK=1588   KO=-     )
> Mean response time                                    34 (OK=34     KO=-     )
> Std. deviation                                        70 (OK=70     KO=-     )
> response time 90th percentile                         38 (OK=38     KO=-     )
> response time 95th percentile                         43 (OK=43     KO=-     )
> response time 99th percentile                         59 (OK=59     KO=-     )
> response time 99.9th percentile                     1588 (OK=1588   KO=-     )
> Mean requests/second                              29.119 (OK=29.119 KO=-     )

S11001001
S11001001 approved these changes Apr 26, 2021
View reviewed changes
Copy link
Contributor

@S11001001 S11001001 left a comment

Choose a reason for hiding this comment

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

Approved modulo included suggestion.

ledger-service/db-backend/src/main/scala/com/digitalasset/http/dbbackend/Queries.scala Outdated Show resolved Hide resolved
@stefanobaghino-da @S11001001
Update ledger-service/db-backend/src/main/scala/com/digitalasset/http…
99c1deb 
…/dbbackend/Queries.scala

Co-authored-by: Stephen Compall <[email protected]>
@stefanobaghino-da
Copy link
Contributor Author

b285fae with #9492 (no extra indices):

---- SyncQueryRequest ----------------------------------------------------------
> Number of requests                                   500 (OK=500    KO=-     )
> Min. response time                                    22 (OK=22     KO=-     )
> Max. response time                                  1588 (OK=1588   KO=-     )
> Mean response time                                    34 (OK=34     KO=-     )
> Std. deviation                                        70 (OK=70     KO=-     )
> response time 90th percentile                         38 (OK=38     KO=-     )
> response time 95th percentile                         43 (OK=43     KO=-     )
> response time 99th percentile                         59 (OK=59     KO=-     )
> response time 99.9th percentile                     1588 (OK=1588   KO=-     )
> Mean requests/second                              29.119 (OK=29.119 KO=-     )

So, if both schemas don't have indices, flattening the stakeholders in the contracts table improves the performance, right?

@stefanobaghino-da stefanobaghino-da merged commit 3e66611 into main Apr 27, 2021
@stefanobaghino-da stefanobaghino-da deleted the ste/9388-2 branch April 27, 2021 10:27
@stefanobaghino-da stefanobaghino-da mentioned this pull request Apr 27, 2021
Closed
@cocreature
Copy link
Contributor

Nice work! Simpler and faster

azure-pipelines bot pushed a commit that referenced this pull request Apr 28, 2021
release 1.13.0-snapshot.20210428.6782.0.e1e878a5
a9ed266 
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.

@nickchapman-da is in charge of this release.

Commit log:
```
e1e878a Simplify opt-in/out of Oracle when building (#9515)
d761853 KVL-874 Add unit tests for Telemetry (#9500)
c567680 Support rollback nodes in PostCommitValidation (#9501)
a335ee8 Support rollback nodes in KeyValueCommitting.submissionOutputs (#9512)
8747b3d Support rollback nodes in TransactionIndexing (#9506)
c4cf3c9 kvutils: Log a missing input state warning without the stack trace. (#9513)
4e712a0 add oracle option for json-api perf runner (#9492)
8cd3658 Switch to an environment variable for enabling Oracle tests. (#9511)
e39c20e update GPG public key (#9488)
782109d update LATEST (#9508)
3e66611 Nest stakeholders in contracts table as JSON arrays (#9484)
aecdc2a update NOTICES file (#9507)
```
Changelog:
```
- [JSON-API Perf] ``--query-store-index=postgres`` must be passed
  to select PostgreSQL query store performance testing; ``true``
  and ``yes`` are no longer supported.
  See `issue #9492 <https://github.com/digital-asset/daml/pull/9492>`__.
```

CHANGELOG_BEGIN
CHANGELOG_END
S11001001 added a commit that referenced this pull request Jun 16, 2021
@S11001001
join and query the contract_stakeholders table for party-set membership
00b8969 
- restoring a few elements removed by 3e66611 (#9484)
@S11001001 S11001001 mentioned this pull request Jun 17, 2021
Merged
6 tasks
S11001001 added a commit that referenced this pull request Jun 17, 2021
@S11001001
index json-api signatories and observers on Oracle (#9895)
3113702 
* nvarchar2 keys are text-incompatible, but varchar2 keys are fine

* commit the ACS update before query

* add changelog

CHANGELOG_BEGIN
- [JSON API] The Oracle database schema has changed; if using
  ``--query-store-jdbc-config``, you must rebuild the database by adding
  ``,createSchema=true``.  See #9895.
CHANGELOG_END

* expand the InitDdl set to include materialized views

* replace search index with a materialized view that expands the stakeholders

* allow materialized views to be created in Oracle testing

* join and query the contract_stakeholders table for party-set membership

- restoring a few elements removed by 3e66611 (#9484)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@S11001001 S11001001 S11001001 approved these changes

@cocreature cocreature Awaiting requested review from cocreature

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stefanobaghino-da @S11001001 @cocreature