-
Notifications
You must be signed in to change notification settings - Fork 403
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
Add txObservers and make use of TXN Stats Collector object #943
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing PR, long needed!!
I had some minor comments, but only the MessengerInterface
ones are in the must-fix category.
@@ -37,13 +37,19 @@ const keys = { | |||
Transparency: 'caliper-report-charting-transparency' | |||
} | |||
}, | |||
Progress: { | |||
Reporting: { | |||
Enabled: 'caliper-progress-reporting-enabled', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enabled
flag could be derived from interval !== 0
. But I'm also fine with this explicit flag.
packages/caliper-core/lib/worker/tx-observers/internal-tx-observer.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/lib/worker/tx-observers/tx-observer-interface.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/lib/worker/tx-observers/tx-observer-dispatch.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/lib/worker/tx-observers/internal-tx-observer.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/lib/manager/monitors/monitor-interface.js
Outdated
Show resolved
Hide resolved
811fac9
to
e9aa4fc
Compare
This pull request introduces 1 alert when merging e9aa4fc into c84d59f - view on LGTM.com new alerts:
|
e9aa4fc
to
0b9613f
Compare
0b9613f
to
bda6a64
Compare
BIG change here brings in transaction observer modules, and with it some other somewhat significant breaking changes that will need equally significant documentation changes:
Transaction observers are specified within the benchmark config file to be used by workers. They may use:
An internal observer is always used, which is used to:
caliper-progress-reporting-interval
(in ms)caliper-progress-reporting-enabled
(true/false)The
local-observer
is renamed todefault-observer
.MessageHandler is renamed WorkerMessageHandler
All transaction statistics are collected, passed and manipulated via the
TransactionStatisticsCollector
class.Workers no longer build an array of transactions to
Promise.all
in order to assure completion, we monitor for completion by taking advantage of aggregated results in the TransactionStatisticsCollector instance.Rate controllers use the worker TransactionStatisticsCollector instance to determine required sleep times for rate control purposes
FixedBacklog controller renamed FixedLoad, since it aims to maintain a fixed txn load on the SUT
Updated generators to set different default values for txDuration and txNumber tests, as well as reduce the number of default workers (mainly to help the build).
Resource manager specification has been changed to mirror the specification of txStatsCollectors. This unifies the specification within the configuration file. A complete specification for monitors (transaction and resource, as per the integration tests) is:
closes #884
closes #826
closes #645
Signed-off-by: [email protected] [email protected]