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

Ledger API Test Tool: add a --max-connection-attempts command line option [KVL-977] #10297

Merged
merged 7 commits into from
Jul 19, 2021
Merged

Ledger API Test Tool: add a --max-connection-attempts command line option [KVL-977] #10297

merged 7 commits into from
Jul 19, 2021

Conversation

fabiotudone-da
Copy link
Contributor

@fabiotudone-da fabiotudone-da commented Jul 15, 2021 •
edited
Loading

Description

This actually originates from failed runs against ledgers that take a long time to spin up:

2021-07-15 10:34:18.341 CEST
conformance-test
Picked up JAVA_TOOL_OPTIONS: -XX:+UseContainerSupport -XX:InitialRAMPercentage=75.0 -XX:MinRAMPercentage=75.0 -XX:MaxRAMPercentage=75.0
Error
2021-07-15 10:34:23.159 CEST
conformance-test
2021-07-15T08:34:23.158Z INFO c.d.l.a.t.LedgerApiTestTool@[main] - Setting up managed channel to participant at localhost:6865...
Error
2021-07-15 10:34:30.859 CEST
conformance-test
2021-07-15T08:34:30.859Z INFO c.d.l.a.t.i.p.ParticipantSession@[kka.actor.default-dispatcher-6] - Could not connect to the participant (attempt #1). Trying again in 100 milliseconds...
...
2021-07-15T08:35:22.066Z INFO c.d.l.a.t.i.p.ParticipantSession@[kka.actor.default-dispatcher-6] - Could not connect to the participant (attempt #10). Trying again in 51200 milliseconds...
Error
2021-07-15 10:35:22.068 CEST
conformance-test
2021-07-15T08:35:22.068Z ERROR c.d.l.a.t.LedgerApiTestTool@[pool-1-thread-1] - Could not connect to the participant: UNAVAILABLE: io exception

CHANGELOG_BEGIN

  • [Integration Kit] Add a --max-connection-attempts command line option to the Ledger API Test Tool.

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.

@fabiotudone-da fabiotudone-da requested review from miklos-da, hubert-da and a user July 15, 2021 15:07
@fabiotudone-da
Add a --max-connection-attempts command line option to the Ledger A…
6be53a9 
…PI Test Tool

BEGIN_CHANGELOG

- [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool

END_CHANGELOG
@fabiotudone-da fabiotudone-da force-pushed the fabiotudone-da/ledger-api-test-tool/allow-configuring-participant-connection-timeout branch from f57520f to 6be53a9 Compare July 15, 2021 15:09
fabiotudone-da
fabiotudone-da commented Jul 15, 2021
View reviewed changes
ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/Cli.scala
.optional()
.text("""Addresses of the participants to test, specified as `<host>:<port>`.""")

arg[Int]("max-connection-attempts")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or just connection-attempts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

max is finer to me :)

@fabiotudone-da
fmt
2d0cd94
@fabiotudone-da
Restore .unbounded() in [endpoints...]
0a8e6c1 
BEGIN_CHANGELOG
- [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool
END_CHANGELOG
@fabiotudone-da
Remove stale comment
988a0a1 
CHANGELOG_BEGIN
- [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool
CHANGELOG_END
ghost
ghost approved these changes Jul 15, 2021
View reviewed changes
.../main/scala/com/daml/ledger/api/testtool/infrastructure/participant/ParticipantSession.scala
ledgerId <- RetryStrategy
.exponentialBackoff(10, 100.millis) { (attempt, wait) =>
.exponentialBackoff(attempts = maxConnectionAttempts, 100.millis) { (attempt, wait) =>
Copy link

Choose a reason for hiding this comment

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

Should we cap this, rather than letting it double every time?

Perhaps to 30 seconds or 1 minute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it does IMO, so that wait times don't become unbearable. I'll try and see if it can be done relatively easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought, I'd merge as soon as the build passes and tackle it in a subsequent PR.

fabiotudone-da and others added 2 commits July 15, 2021 18:35
@fabiotudone-da @miklos-da
Update ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api…
acd4544 
…/testtool/infrastructure/LedgerTestCasesRunner.scala

Co-authored-by: Miklos <57664299+miklos-da@users.noreply.github.com>
@fabiotudone-da
fmt
1a944f2
@mergify mergify bot merged commit d69328d into main Jul 19, 2021
@mergify mergify bot deleted the fabiotudone-da/ledger-api-test-tool/allow-configuring-participant-connection-timeout branch July 19, 2021 09:52
@fabiotudone-da fabiotudone-da mentioned this pull request Jul 19, 2021
Merged
6 tasks
azure-pipelines bot pushed a commit that referenced this pull request Jul 21, 2021
release 1.16.0-snapshot.20210720.7404.0.b7cf42d1
0e3bbe1 
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.

@Robin-da is in charge of this release.

Commit log:
```
b7cf42d Upgrade doobie to version 0.13.4 (#10326)
1a72314 fix broken triggers link (#10336)
271f9da Added back DeduplicationPeriod.DeduplicationOffset [KVL-998] (#10324)
b701caa Report divulgence warning at commit location. (#10329)
742bdcb Describe prometheus in daml-on-sql README (#10307)
93d0ed6 [Mutable cache] Resolve with full lookup on negative cache read-through lookups [DPP-501] (#10262)
f59951b document min npm version (#10330)
d982e16 fix besu listing (#10328)
c3bc26f daml-lf/data: Move ID aliases to `Ref` from _ledger-api-common_. [KVL-1002] (#10323)
cf2f79d Register divulgence warning as a diagnostic (#10289)
8360e9f Mark Java Bot tests flaky (#10322)
a6bf892 [In-memory fan-out] CLI parameters update (#10250)
5fa49c3 Disables flaky test suite of InstrumentedSource.bufferedSource (#10321)
504c231 Use a combined template id, stakeholder index (#10315)
9a01065 participant-state: Remove the Daml-LF transaction and value aliases. [KVL-1002] (#10317)
159728d LF: use Either by default in all archive reader API (#10295)
449a72a increase ES memory (#10318)
80b65f6 [DOCS] Add documentation for the JSON API metrics (#10312)
6abb903 [JSON-API] Put the request & response bodies into the log ctx if the log level is debug (for specific statements) (#10298)
3c0010b detect mount issue earlier (#10313)
0e09f3b Remove optLocation field from transaction nodes. (#10301)
e62852f Make ledger API test tool's 'max-connection-attempts' into an option (#10311)
bd12170 participant-state: Remove the `ParticipantId`, `PackageId`, and `Party` aliases. [KVL-1002] (#10308)
fed4497 Extend JSON API ContractDao query bench’s with different tpids (#10309)
bd35f80 ledger-configuration: Correct the protobuf package. [KVL-1002] (#10302)
d69328d Ledger API Test Tool: add a `--max-connection-attempts` command line option [KVL-977] (#10297)
a3b861e refresh es cluster (#10300)
63332d0 update NOTICES file (#10306)
1ee53c0 participant-state-index: Use `Ref` directly. (#10299)
0eba812 Remove trace_context field from Ledger API [KVL-1021] (#10256)
88886be ledger-configuration: Add tests for the `Configuration` decoding methods. [KVL-1002] (#10293)
cb29f34 ledger-offset: Move `Offset` to a new package. [KVL-1002] (#10296)
b899ae1 Codeowners: Add @nicu-da to the kvutils code review. (#10292)
f033bc2 LF: Clean legacy from archive proto + TEXT_TO_CODE_POINTS  typo (#10286)
70b12f0 ledger-on-sql: Remove conformance tests for H2 and SQLite files. (#10291)
a9a0b70 ledger-configuration: Extract the configuration from participant-state. [KVL-1002] (#10279)
d96b54e Use Text not String in exception docs (#10287)
5382253 Refresh json API materialized view on statement (#10285)
6e447c5 Patch export filtering of GHC.Types.[] in damldocs (#10282)
d565cae DPP-457 Add UpdateToDbdto tests (#10116)
814442e update NOTICES file (#10284)
0493480 rotate release duty after 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10268)
84094e0 update compat versions for 1.15.0 (#10281)
ba8e829 [In-memory fan-out] Oracle support (#10263)
a8f1902 LF: change type from Try to Either in archive module (#10277)
c84b37a Release SDK 1.15 (#10269)
980d96b Fix status check in collect_build_data (#10278)
191d3b8 Add unit test to SequentialWriteDao [DPP-456] (#10265)
2271f93 Fix crash on duplicate submissionId (#10242)
906184c LF: Simplify UniversalDarReader (#10271)
42c487b [JSON-API] Refactor Endpoints.scala to use path directives etc. (#10274)
6840401 release 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10267)
0df70fa Document bazel-testlogs directory in BAZEL.md (#10276)
44204bb Add a benchmark for contract insertion in the JSON API (#10272)
6107f8a Ignore failure to upload log failures (#10270)
caf85a2 LF: rationalize archive Parser/Reader/Decoder (#10239)
0043b81 Add a ContractDao benchmark (#10259)
21a9e62 Add disclaimer to participant state v2 API [KVL-998] (#10254)
6c63f96 participant-integration-api: Move `SeedService` here. (#10261)
58c0b46 kvutils: Simplify usage of Rejections [KVL-1015] (#10258)
41aba1c kvutils: Move unrelated test cases to a proper test class [KVL-1015] (#10257)
```
Changelog:
```

- [JSON-API] You can now find a section `Metrics` in the http-json api documentation explaining how to enable metrics and which are available

- [JSON-API] Logging of the request and response bodies are now available for appropriate requests if the chosen log level is equal or lower than DEBUG. These can then be found in the logging context of the request begin & end log messages (The field names in the ctx are "request_body" and "response_body").
- [Integration Kit] The *ledger_configuration.proto* Protobuf definition
  has been repackaged under ``com.daml.ledger.configuration``, and the
  Java and C# packages have been renamed accordingly. If you are using
  this Protobuf definition, you will need to update your imports. The
  Maven artifact name has not changed.
- [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool
* [Integration Kit] Removed trace_context field from Ledger API and its bindings as we now have trace context propagation support via gRPC metadata. If you are constructing or consuming Ledger API requests or responses directly, you may need to update your code.
- [Integration Kit] The ``Offset`` type has been moved to a new Maven
  package, ``ledger-offset``, from the ``participant-state`` package.
  The Java package has been renamed to ``com.daml.ledger.offset``. If
  you are using this type, you will need to update your dependencies and
  imports.
- [Integration Kit] The ledger configuration classes, ``Configuration``,
  ``LedgerInitialConditions``, and ``TimeModel``, have been moved from
  *participant-state* to a separate package named
  *ledger-configuration*, in the Java package
  ``com.daml.ledger.configuration``. You will need to update your
  dependencies and imports.
- [Integration Kit] ``TimeModel`` has been renamed to
  ``LedgerTimeModel``. If you are using the ledger configuration classes
  directly, you may need to update your code.
Fix crash in ConfigManagement- and PackageManagement- services on duplicate submissionsIds from different participants.
Extend ledger-api-test-tool to cover the duplicate submissionId cases.
- [Integration Kit] The class ``SeedService`` has been moved from the
  *participant-state* Maven package to the *participant-integration-api*
  Maven package, under the Java package name
  ``com.daml.platform.apiserver`` to reflect its usage by the API
  server, not the participant state API. If you use this class directly,
  you will need to change your imports.
```

CHANGELOG_BEGIN
CHANGELOG_END
ghost pushed a commit that referenced this pull request Jul 21, 2021
@azure-pipelines
release 1.16.0-snapshot.20210720.7404.0.b7cf42d1 (#10345)
186d279 
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.

@Robin-da is in charge of this release.

Commit log:
```
b7cf42d Upgrade doobie to version 0.13.4 (#10326)
1a72314 fix broken triggers link (#10336)
271f9da Added back DeduplicationPeriod.DeduplicationOffset [KVL-998] (#10324)
b701caa Report divulgence warning at commit location. (#10329)
742bdcb Describe prometheus in daml-on-sql README (#10307)
93d0ed6 [Mutable cache] Resolve with full lookup on negative cache read-through lookups [DPP-501] (#10262)
f59951b document min npm version (#10330)
d982e16 fix besu listing (#10328)
c3bc26f daml-lf/data: Move ID aliases to `Ref` from _ledger-api-common_. [KVL-1002] (#10323)
cf2f79d Register divulgence warning as a diagnostic (#10289)
8360e9f Mark Java Bot tests flaky (#10322)
a6bf892 [In-memory fan-out] CLI parameters update (#10250)
5fa49c3 Disables flaky test suite of InstrumentedSource.bufferedSource (#10321)
504c231 Use a combined template id, stakeholder index (#10315)
9a01065 participant-state: Remove the Daml-LF transaction and value aliases. [KVL-1002] (#10317)
159728d LF: use Either by default in all archive reader API (#10295)
449a72a increase ES memory (#10318)
80b65f6 [DOCS] Add documentation for the JSON API metrics (#10312)
6abb903 [JSON-API] Put the request & response bodies into the log ctx if the log level is debug (for specific statements) (#10298)
3c0010b detect mount issue earlier (#10313)
0e09f3b Remove optLocation field from transaction nodes. (#10301)
e62852f Make ledger API test tool's 'max-connection-attempts' into an option (#10311)
bd12170 participant-state: Remove the `ParticipantId`, `PackageId`, and `Party` aliases. [KVL-1002] (#10308)
fed4497 Extend JSON API ContractDao query bench’s with different tpids (#10309)
bd35f80 ledger-configuration: Correct the protobuf package. [KVL-1002] (#10302)
d69328d Ledger API Test Tool: add a `--max-connection-attempts` command line option [KVL-977] (#10297)
a3b861e refresh es cluster (#10300)
63332d0 update NOTICES file (#10306)
1ee53c0 participant-state-index: Use `Ref` directly. (#10299)
0eba812 Remove trace_context field from Ledger API [KVL-1021] (#10256)
88886be ledger-configuration: Add tests for the `Configuration` decoding methods. [KVL-1002] (#10293)
cb29f34 ledger-offset: Move `Offset` to a new package. [KVL-1002] (#10296)
b899ae1 Codeowners: Add @nicu-da to the kvutils code review. (#10292)
f033bc2 LF: Clean legacy from archive proto + TEXT_TO_CODE_POINTS  typo (#10286)
70b12f0 ledger-on-sql: Remove conformance tests for H2 and SQLite files. (#10291)
a9a0b70 ledger-configuration: Extract the configuration from participant-state. [KVL-1002] (#10279)
d96b54e Use Text not String in exception docs (#10287)
5382253 Refresh json API materialized view on statement (#10285)
6e447c5 Patch export filtering of GHC.Types.[] in damldocs (#10282)
d565cae DPP-457 Add UpdateToDbdto tests (#10116)
814442e update NOTICES file (#10284)
0493480 rotate release duty after 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10268)
84094e0 update compat versions for 1.15.0 (#10281)
ba8e829 [In-memory fan-out] Oracle support (#10263)
a8f1902 LF: change type from Try to Either in archive module (#10277)
c84b37a Release SDK 1.15 (#10269)
980d96b Fix status check in collect_build_data (#10278)
191d3b8 Add unit test to SequentialWriteDao [DPP-456] (#10265)
2271f93 Fix crash on duplicate submissionId (#10242)
906184c LF: Simplify UniversalDarReader (#10271)
42c487b [JSON-API] Refactor Endpoints.scala to use path directives etc. (#10274)
6840401 release 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10267)
0df70fa Document bazel-testlogs directory in BAZEL.md (#10276)
44204bb Add a benchmark for contract insertion in the JSON API (#10272)
6107f8a Ignore failure to upload log failures (#10270)
caf85a2 LF: rationalize archive Parser/Reader/Decoder (#10239)
0043b81 Add a ContractDao benchmark (#10259)
21a9e62 Add disclaimer to participant state v2 API [KVL-998] (#10254)
6c63f96 participant-integration-api: Move `SeedService` here. (#10261)
58c0b46 kvutils: Simplify usage of Rejections [KVL-1015] (#10258)
41aba1c kvutils: Move unrelated test cases to a proper test class [KVL-1015] (#10257)
```
Changelog:
```

- [JSON-API] You can now find a section `Metrics` in the http-json api documentation explaining how to enable metrics and which are available

- [JSON-API] Logging of the request and response bodies are now available for appropriate requests if the chosen log level is equal or lower than DEBUG. These can then be found in the logging context of the request begin & end log messages (The field names in the ctx are "request_body" and "response_body").
- [Integration Kit] The *ledger_configuration.proto* Protobuf definition
  has been repackaged under ``com.daml.ledger.configuration``, and the
  Java and C# packages have been renamed accordingly. If you are using
  this Protobuf definition, you will need to update your imports. The
  Maven artifact name has not changed.
- [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool
* [Integration Kit] Removed trace_context field from Ledger API and its bindings as we now have trace context propagation support via gRPC metadata. If you are constructing or consuming Ledger API requests or responses directly, you may need to update your code.
- [Integration Kit] The ``Offset`` type has been moved to a new Maven
  package, ``ledger-offset``, from the ``participant-state`` package.
  The Java package has been renamed to ``com.daml.ledger.offset``. If
  you are using this type, you will need to update your dependencies and
  imports.
- [Integration Kit] The ledger configuration classes, ``Configuration``,
  ``LedgerInitialConditions``, and ``TimeModel``, have been moved from
  *participant-state* to a separate package named
  *ledger-configuration*, in the Java package
  ``com.daml.ledger.configuration``. You will need to update your
  dependencies and imports.
- [Integration Kit] ``TimeModel`` has been renamed to
  ``LedgerTimeModel``. If you are using the ledger configuration classes
  directly, you may need to update your code.
Fix crash in ConfigManagement- and PackageManagement- services on duplicate submissionsIds from different participants.
Extend ledger-api-test-tool to cover the duplicate submissionId cases.
- [Integration Kit] The class ``SeedService`` has been moved from the
  *participant-state* Maven package to the *participant-integration-api*
  Maven package, under the Java package name
  ``com.daml.platform.apiserver`` to reflect its usage by the API
  server, not the participant state API. If you use this class directly,
  you will need to change your imports.
```

CHANGELOG_BEGIN
CHANGELOG_END

Co-authored-by: Azure Pipelines DAML Build <support@digitalasset.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hubert-da hubert-da hubert-da approved these changes

@miklos-da miklos-da miklos-da approved these changes

@gerolf-da gerolf-da Awaiting requested review from gerolf-da

@mziolekda mziolekda Awaiting requested review from mziolekda

@nmarton-da nmarton-da Awaiting requested review from nmarton-da

@rautenrieth-da rautenrieth-da Awaiting requested review from rautenrieth-da

@tudor-da tudor-da Awaiting requested review from tudor-da

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
@fabiotudone-da @miklos-da @hubert-da