ledger-api-bench-tool - min consumption speed SLO [DPP-401]
#9808
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kamil-da
requested review from
gerolf-da,
mziolekda,
nmarton-da,
rautenrieth-da and
tudor-da
as code owners
tudor-da
reviewed
May 27, 2021
| val updatedFirstRecordTime = | ||
| firstRecordTime match { | ||
| val newPreviousLatest = | ||
| previousLatest match { | ||
| case None => | ||
| recordTimes.headOption.map { recordTime => |
There was a problem hiding this comment.
I didn't check where this is coming from, but is recordTimes sorted descending or does it grow like a stack? It's a bit confusing to see the head value named as ..latest
There was a problem hiding this comment.
From what I understand streams served by a ledger have ascending record times.
I've redefined the ConsumptionSpeecMetric a bit. Previously we computed a periodic speed like:
periodicSpeed = (lastRecordTimeInPeriod - firstRecordTimeInPeriod) / periodDuration. This definition was problematic in case of a single element in a period or no elements at all.
Currently the definition is: periodicSpeed = (previousLastConsideredElement - lastConsideredElement) / periodDuration. The previousLastConsideredElement may be in the current period (for the first period) or in the most recent previous period with elements.
I'll update the readme file with detailed description of this.
There was a problem hiding this comment.
Readme updates in #9822
Base automatically changed from
kamil-da/DPP-401-bench-tool-min-consumption-speed-SLO
to
main
May 27, 2021 10:12
kamil-da
force-pushed
the
kamil-da/DPP-401-bench-tool-min-consumption-speed-SLO-2
branch
from
8825945 to
4c08099
Compare
CHANGELOG_BEGIN CHANGELOG_END
CHANGELOG_BEGIN - [Integration Kit] - ledger-api-bench-tool - new consumption speed service-level objective for the ledger-api-bench-tool, configurable with a CLI parameter --min-consumption-speed CHANGELOG_END
kamil-da
force-pushed
the
kamil-da/DPP-401-bench-tool-min-consumption-speed-SLO-2
branch
from
4c08099 to
5054869
Compare
mziolekda
reviewed
May 27, 2021
| Some((last.toEpochMilli - first.toEpochMilli) * 1.0 / periodMillis) | ||
| (previousLatest, currentPeriodLatest) match { | ||
| case (Some(previous), Some(current)) => | ||
| Some((current.toEpochMilli - previous.toEpochMilli).toDouble / periodMillis) |
There was a problem hiding this comment.
There is no guarantee that the periodMillis is being maintained by the thread scheduler. It is safer to assume it is not and rely on a current time, to facilitate the unit testing you may want to supply last period as argument of periodicValue(periodDuration: Duration)
There was a problem hiding this comment.
Good catch! I'll fix this metric and others that may rely on the constant period duration.
mziolekda
reviewed
May 27, 2021
| finalValue shouldBe ConsumptionSpeedMetric.Value(None) | ||
| } | ||
|
|
||
| "correctly handle periods with no elements" in { |
There was a problem hiding this comment.
Add a unit test in a situation where periodicValue is not called in a timely fashion i.e. frequency varies up and down
mziolekda
approved these changes
May 27, 2021
|
Requested changes will be addressed in #9822. |
kamil-da
deleted the
kamil-da/DPP-401-bench-tool-min-consumption-speed-SLO-2
branch
azure-pipelines bot
pushed a commit
that referenced
this pull request
Jun 2, 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. @cocreature is in charge of this release. Commit log: ``` 7d716e6 Try to unflake JSON API failure tests (#9855) 2d49939 Fix the notion of transaction equivalence in the ledger model for exceptions. (#9850) fd8f8a6 Daml-lf suffixCid ignores V0 contract ids rather than erroring (#9867) e38bb4a Make data-deps exception tests version-aware. (#9870) 94129b2 Add tests for decoding of UpdateTryCatch (#9865) 1cc0906 Adda test for an unhandled builtin exception to ExceptionTest.scala (#9866) d7c60f5 resources: Only mutate test mock objects inside `synchronized` blocks. (#9869) 3a0b2f9 remove 2x 8020 todo tags (#9868) 1595f06 Special case _tryCatch @update in LFConversion. (#9864) c0359ac Add trace context propagation in CommandClient.trackCommandsUnbounded [KVL-961] (#9833) 2a1e1ba Add a Foldable instance for Set (#9860) 590b8b3 Reshuffle tests for AnyExceptionMessage (#9863) 89bd478 Reenable AnyException tests in ExceptionTest.scala (#9861) 1573984 Include rollback nodes in danglingRefGenNode (#9851) e5d4553 Remove flyway warning when using the append-only schema (#9840) a4e33c9 Add metrics to parallel indexer (#9841) 972af2a Test To/FromAnyException primitives in DecodeV1Spec (#9852) 5f4f019 update NOTICES file (#9859) f5f2a8a Remove obsolete transaction todo (#9853) 8fc4543 Uniform configuration of the command service across all ledgers (#9839) 1f4a958 ledger-api-bench-tool - single-value metrics (#9822) 6a1c797 ledger-api-bench-tool: TLS support (#9824) 5528347 Test TransactionsReader.getContractStateEvents within the JDBC DAO suite (#9849) 6af6c93 Fix behavior of Math.sqrt at 0 (#9818) (#9828) 7855ea5 Switch to Ubuntu 20.04 nodes for ghc lib (#9838) c0fccfc Switch Scala version Bazel config (#9848) e033277 Switch to our own nodes for releases (#9845) b6a89a6 Bump ws to address security advisory (#9844) 25b7e54 Only resolve template id once (#9837) a645971 Enable mutable-contract-state-cache for ledger-on-SQL append-only tests (#9836) dcd33a7 Add a section on exceptions to the Daml intro (#9832) 859cae3 [Mutable contract state cache] Tests for contract/key state new methods in ContractsReader (#9825) ffe5586 Track oldChild in PartialTransaction context (#9815) 6e17e2d [In memory fan-out] Transaction log updates stream implementation (#9792) a0373c4 Ledger model section on transaction normalization. (#9816) f355931 Fix typeclass detection in data-dependencies (#9830) 9626a59 Speedy: Clean SBuiltin (#9829) 269237e Fix authorization in try contexts (#9823) cb06fb3 `ledger-api-bench-tool` - min consumption speed SLO [DPP-401] (#9808) 0868324 update NOTICES file (#9827) a166e79 Added maxlength information to id string (#9826) 63bc0d1 EventsBuffer implementation for in-memory fan-out for Ledger API serving (#9775) 0c12259 Speedy: Use SAny instead of SAnyException to reperesent AnyExcpetion (#9819) 6caa61c participant-state: rejection reason abstractions (#9764) 63c471f Upgrade protobuf-java and scalapb. [KVL-938] (#9713) f177c1a `ledger-api-bench-tool` - Moved classes to individual files [DPP-401] (#9803) a3b10dc LF: fix parser (#9809) 9f55da8 update NOTICES file (#9812) 6846dbf update compat versions for 1.14.0-snapshot.20210526.7024.0.aedb9a82 (#9806) 1f021b2 LF: Drop Builtin Exceptions completly (#9790) ea5b710 rotate release duty after 1.14.0-snapshot.20210525.7017.0.2710fad0 (#9795) 0c7b149 Add reference docs for exceptions (#9807) 6b67ba0 LF: SBAnyExceptionMessage queries for unknown packages. (#9804) fdab1e6 Parallelize daml-assistant integration tests. (#9802) 2e9c56e DPP-393 Activate JdbcLedgerDao suite for the append-only schema (#9793) 274fcae release 1.14.0-snapshot.20210525.7017.0.2710fad0 (#9801) 3241ad6 `ledger-api-bench-tool` - updated metrics reporting model [DPP-400] (#9749) ``` Changelog: ``` Modify command line options of the ledgers based on the kvutils/app library: - Remove max-commands-in-flight sub-option from the participant group. - Add command service configuration options in line with sandbox and daml-on-sql: max-commands-in-flight, max-parallel-submissions, input-buffer-size - [Integration Kit] - TLS support for the ledger-api-bench-tool [Daml Compiler] Ensure that sqrt(0.0) == 0 and 0 ** 0 == 1 - [Integration Kit] - ledger-api-bench-tool - new consumption speed service-level objective for the ledger-api-bench-tool, configurable with a CLI parameter --min-consumption-speed - Participant-state: Introduced versioned form of rejection reason - [Integration Kit] - ledger-api-bench-tool - new parameter max-delay for specifying the service-level objective for max delay ``` CHANGELOG_BEGIN CHANGELOG_END
cocreature
pushed a commit
that referenced
this pull request
Jun 2, 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. @cocreature is in charge of this release. Commit log: ``` 7d716e6 Try to unflake JSON API failure tests (#9855) 2d49939 Fix the notion of transaction equivalence in the ledger model for exceptions. (#9850) fd8f8a6 Daml-lf suffixCid ignores V0 contract ids rather than erroring (#9867) e38bb4a Make data-deps exception tests version-aware. (#9870) 94129b2 Add tests for decoding of UpdateTryCatch (#9865) 1cc0906 Adda test for an unhandled builtin exception to ExceptionTest.scala (#9866) d7c60f5 resources: Only mutate test mock objects inside `synchronized` blocks. (#9869) 3a0b2f9 remove 2x 8020 todo tags (#9868) 1595f06 Special case _tryCatch @update in LFConversion. (#9864) c0359ac Add trace context propagation in CommandClient.trackCommandsUnbounded [KVL-961] (#9833) 2a1e1ba Add a Foldable instance for Set (#9860) 590b8b3 Reshuffle tests for AnyExceptionMessage (#9863) 89bd478 Reenable AnyException tests in ExceptionTest.scala (#9861) 1573984 Include rollback nodes in danglingRefGenNode (#9851) e5d4553 Remove flyway warning when using the append-only schema (#9840) a4e33c9 Add metrics to parallel indexer (#9841) 972af2a Test To/FromAnyException primitives in DecodeV1Spec (#9852) 5f4f019 update NOTICES file (#9859) f5f2a8a Remove obsolete transaction todo (#9853) 8fc4543 Uniform configuration of the command service across all ledgers (#9839) 1f4a958 ledger-api-bench-tool - single-value metrics (#9822) 6a1c797 ledger-api-bench-tool: TLS support (#9824) 5528347 Test TransactionsReader.getContractStateEvents within the JDBC DAO suite (#9849) 6af6c93 Fix behavior of Math.sqrt at 0 (#9818) (#9828) 7855ea5 Switch to Ubuntu 20.04 nodes for ghc lib (#9838) c0fccfc Switch Scala version Bazel config (#9848) e033277 Switch to our own nodes for releases (#9845) b6a89a6 Bump ws to address security advisory (#9844) 25b7e54 Only resolve template id once (#9837) a645971 Enable mutable-contract-state-cache for ledger-on-SQL append-only tests (#9836) dcd33a7 Add a section on exceptions to the Daml intro (#9832) 859cae3 [Mutable contract state cache] Tests for contract/key state new methods in ContractsReader (#9825) ffe5586 Track oldChild in PartialTransaction context (#9815) 6e17e2d [In memory fan-out] Transaction log updates stream implementation (#9792) a0373c4 Ledger model section on transaction normalization. (#9816) f355931 Fix typeclass detection in data-dependencies (#9830) 9626a59 Speedy: Clean SBuiltin (#9829) 269237e Fix authorization in try contexts (#9823) cb06fb3 `ledger-api-bench-tool` - min consumption speed SLO [DPP-401] (#9808) 0868324 update NOTICES file (#9827) a166e79 Added maxlength information to id string (#9826) 63bc0d1 EventsBuffer implementation for in-memory fan-out for Ledger API serving (#9775) 0c12259 Speedy: Use SAny instead of SAnyException to reperesent AnyExcpetion (#9819) 6caa61c participant-state: rejection reason abstractions (#9764) 63c471f Upgrade protobuf-java and scalapb. [KVL-938] (#9713) f177c1a `ledger-api-bench-tool` - Moved classes to individual files [DPP-401] (#9803) a3b10dc LF: fix parser (#9809) 9f55da8 update NOTICES file (#9812) 6846dbf update compat versions for 1.14.0-snapshot.20210526.7024.0.aedb9a82 (#9806) 1f021b2 LF: Drop Builtin Exceptions completly (#9790) ea5b710 rotate release duty after 1.14.0-snapshot.20210525.7017.0.2710fad0 (#9795) 0c7b149 Add reference docs for exceptions (#9807) 6b67ba0 LF: SBAnyExceptionMessage queries for unknown packages. (#9804) fdab1e6 Parallelize daml-assistant integration tests. (#9802) 2e9c56e DPP-393 Activate JdbcLedgerDao suite for the append-only schema (#9793) 274fcae release 1.14.0-snapshot.20210525.7017.0.2710fad0 (#9801) 3241ad6 `ledger-api-bench-tool` - updated metrics reporting model [DPP-400] (#9749) ``` Changelog: ``` Modify command line options of the ledgers based on the kvutils/app library: - Remove max-commands-in-flight sub-option from the participant group. - Add command service configuration options in line with sandbox and daml-on-sql: max-commands-in-flight, max-parallel-submissions, input-buffer-size - [Integration Kit] - TLS support for the ledger-api-bench-tool [Daml Compiler] Ensure that sqrt(0.0) == 0 and 0 ** 0 == 1 - [Integration Kit] - ledger-api-bench-tool - new consumption speed service-level objective for the ledger-api-bench-tool, configurable with a CLI parameter --min-consumption-speed - Participant-state: Introduced versioned form of rejection reason - [Integration Kit] - ledger-api-bench-tool - new parameter max-delay for specifying the service-level objective for max delay ``` 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
min-consumption-speedparameterPull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.