Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Per subsystem CPU usage tracking #4239

Merged
merged 28 commits into from
Nov 11, 2021
Merged

Per subsystem CPU usage tracking #4239

merged 28 commits into from
Nov 11, 2021

Conversation

sandreim
Copy link
Contributor

@sandreim sandreim commented Nov 8, 2021 •
edited by ordian
Loading

Partially implement #4198

Sets task_group label of task metrics to allow aggregation based on subsystem. Most of the tasks spawned in the Polkadot codebase will now have an appropriate name and group, but Substrate tasks don't set the group.

I view this as a good starting point that we can iterate on to consistently group tasks in future PRs.

Substrate changes: paritytech/substrate#10196

skip check-dependent-cumulus

@sandreim
SubsystemContext: add subsystem name str
4eafe81 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Overseer builder proc macro changes
255c147 
* initilize SubsystemContext name field.
* Add subsystem name in TaskKind::launch_task()

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Update ToOverseer enum
6dca05c 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Assign subsystem names to orphan tasks
c0a332c 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
cargo fmt
26a6aac 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
SubsystemContext: add subsystem name str
5afa046 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Overseer builder proc macro changes
c88911c 
* initilize SubsystemContext name field.
* Add subsystem name in TaskKind::launch_task()

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Update ToOverseer enum
ba6f57a 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Assign subsystem names to orphan tasks
710941e 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
cargo fmt
4a3bbbd 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Rebase changes for new spawn() group param
d7a4a5e 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Add subsystem constat in JobTrait
61f27e6 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Add subsystem string
5494f18 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Fix tests
266645a 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Fix spawn() calls
38fe1e8 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
cargo fmt
cdfe97a 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
b9f986f 
…/per_subsystem_task_metrics
@sandreim
Merge branch 'sandreim/per_subsystem_task_metrics' of github.com:pari…
610abf4 
…tytech/polkadot into sandreim/per_subsystem_task_metrics

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Fix
81d8af5 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim sandreim added A0-please_review Pull request needs code review. B1-releasenotes C1-low PR touches the given topic and has a low impact on builders. labels Nov 8, 2021
@sandreim sandreim mentioned this pull request Nov 8, 2021
Merged
@ordian
Copy link
Member

ordian commented Nov 8, 2021

The code makes sense to me, but I would prefer more consistent casing: we're using PascalCase for Jobs, snake_case for subsystems/groups and kebab-case for task names. It would be nice to settle on one :)

@sandreim
Fix tests
95c50eb 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
fix
8b8e5dc 
Signed-off-by: Andrei Sandu <[email protected]>
drahnr
drahnr approved these changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@drahnr drahnr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A few nits, generally looks good! The naming still needs a final resolution regarding the right casing :)

node/core/backing/src/lib.rs Outdated Show resolved Hide resolved
node/core/pvf/src/executor_intf.rs Outdated Show resolved Hide resolved
node/core/runtime-api/src/lib.rs Outdated Show resolved Hide resolved
node/core/runtime-api/src/lib.rs Outdated Show resolved Hide resolved
node/jaeger/src/lib.rs Outdated Show resolved Hide resolved
node/overseer/overseer-gen/proc-macro/src/impl_builder.rs Outdated Show resolved Hide resolved
node/overseer/overseer-gen/proc-macro/src/impl_builder.rs Outdated Show resolved Hide resolved
@sandreim
Fix more tests
cf80a1f 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Address PR review feedback #1
393fb66 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Merge branch 'master' of github.com:paritytech/polkadot into sandreim…
3b4d098 
…/per_subsystem_task_metrics

Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Address PR review round 2
5fd25dd 
Signed-off-by: Andrei Sandu <[email protected]>
@sandreim
Fixes
dae09bc 
- remove JobTrait::Subsystem
- fix tests

Signed-off-by: Andrei Sandu <[email protected]>
ordian
ordian approved these changes Nov 11, 2021
View reviewed changes
node/overseer/overseer-gen/proc-macro/src/impl_builder.rs
// Generate subsystem name based on overseer field name.
let mut subsystem_string = String::from(stringify!(#subsystem_name));
// Convert owned `snake case` string to a `kebab case` static str.
let subsystem_static_str = Box::leak(subsystem_string.replace("_", "-").into_boxed_str());
Copy link
Member

Choose a reason for hiding this comment

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

not a fan of this, but seems correct

@paritytech-processbot
Copy link

Companion update failed: CommandFailed { cmd: "Command { std: "cargo" "update" "-v" "-p" "pallet-democracy:4.0.0-dev" "-p" "sc-service:0.10.0-dev" "-p" "sc-client-api:4.0.0-dev" "-p" "pallet-bags-list:4.0.0-dev" "-p" "beefy-gadget-rpc:4.0.0-dev" "-p" "beefy-gadget:4.0.0-dev" "-p" "sp-staking:4.0.0-dev" "-p" "pallet-transaction-payment-rpc:4.0.0-dev" "-p" "sp-trie:4.0.0-dev" "-p" "sc-consensus:0.10.0-dev" "-p" "pallet-mmr-primitives:4.0.0-dev" "-p" "pallet-im-online:4.0.0-dev" "-p" "pallet-proxy:4.0.0-dev" "-p" "sp-runtime-interface-proc-macro:4.0.0-dev" "-p" "sc-consensus-slots:0.10.0-dev" "-p" "sp-api:4.0.0-dev" "-p" "pallet-grandpa:4.0.0-dev" "-p" "pallet-beefy:4.0.0-dev" "-p" "pallet-gilt:4.0.0-dev" "-p" "pallet-sudo:4.0.0-dev" "-p" "sp-application-crypto:4.0.0-dev" "-p" "sp-keyring:4.0.0-dev" "-p" "sp-runtime:4.0.0-dev" "-p" "sp-wasm-interface:4.0.0-dev" "-p" "pallet-election-provider-multi-phase:4.0.0-dev" "-p" "pallet-transaction-payment-rpc-runtime-api:4.0.0-dev" "-p" "sc-basic-authorship:0.10.0-dev" "-p" "sp-core-hashing:4.0.0-dev" "-p" "pallet-beefy-mmr:4.0.0-dev" "-p" "sc-rpc:4.0.0-dev" "-p" "sp-state-machine:0.10.0-dev" "-p" "sc-consensus-babe-rpc:0.10.0-dev" "-p" "beefy-primitives:4.0.0-dev" "-p" "pallet-session:4.0.0-dev" "-p" "pallet-bounties:4.0.0-dev" "-p" "pallet-assets:4.0.0-dev" "-p" "sp-version:4.0.0-dev" "-p" "sc-consensus-babe:0.10.0-dev" "-p" "sc-transaction-pool:4.0.0-dev" "-p" "sp-finality-grandpa:4.0.0-dev" "-p" "sp-maybe-compressed-blob:4.1.0-dev" "-p" "sc-network:0.10.0-dev" "-p" "frame-benchmarking:4.0.0-dev" "-p" "sp-core-hashing-proc-macro:4.0.0-dev" "-p" "sp-storage:4.0.0-dev" "-p" "sp-offchain:4.0.0-dev" "-p" "sc-state-db:0.10.0-dev" "-p" "sp-authority-discovery:4.0.0-dev" "-p" "sc-cli:0.10.0-dev" "-p" "pallet-collective:4.0.0-dev" "-p" "sc-telemetry:4.0.0-dev" "-p" "sc-utils:4.0.0-dev" "-p" "sp-runtime-interface:4.0.0-dev" "-p" "pallet-society:4.0.0-dev" "-p" "substrate-build-script-utils:3.0.0" "-p" "sp-debug-derive:4.0.0-dev" "-p" "sc-client-db:0.10.0-dev" "-p" "sp-rpc:4.0.0-dev" "-p" "pallet-indices:4.0.0-dev" "-p" "frame-system-rpc-runtime-api:4.0.0-dev" "-p" "pallet-mmr:4.0.0-dev" "-p" "sc-peerset:4.0.0-dev" "-p" "frame-support-procedural-tools-derive:3.0.0" "-p" "pallet-utility:4.0.0-dev" "-p" "sp-consensus:0.10.0-dev" "-p" "sp-panic-handler:4.0.0-dev" "-p" "sp-transaction-storage-proof:4.0.0-dev" "-p" "frame-benchmarking-cli:4.0.0-dev" "-p" "sp-tracing:4.0.0-dev" "-p" "try-runtime-cli:0.10.0-dev" "-p" "sc-consensus-manual-seal:0.10.0-dev" "-p" "substrate-wasm-builder:5.0.0-dev" "-p" "sc-tracing:4.0.0-dev" "-p" "frame-support-test:3.0.0" "-p" "sp-io:4.0.0-dev" "-p" "sp-serializer:4.0.0-dev" "-p" "pallet-staking-reward-curve:4.0.0-dev" "-p" "sp-api-proc-macro:4.0.0-dev" "-p" "sp-version-proc-macro:4.0.0-dev" "-p" "sc-network-gossip:0.10.0-dev" "-p" "pallet-transaction-payment:4.0.0-dev" "-p" "frame-support-procedural-tools:4.0.0-dev" "-p" "pallet-mmr-rpc:3.0.0" "-p" "frame-support-procedural:4.0.0-dev" "-p" "frame-support-test-pallet:4.0.0-dev" "-p" "sc-keystore:4.0.0-dev" "-p" "sc-tracing-proc-macro:4.0.0-dev" "-p" "sp-npos-elections:4.0.0-dev" "-p" "pallet-offences-benchmarking:4.0.0-dev" "-p" "sc-block-builder:0.10.0-dev" "-p" "sc-authority-discovery:0.10.0-dev" "-p" "sp-authorship:4.0.0-dev" "-p" "sp-npos-elections-solution-type:4.0.0-dev" "-p" "sc-allocator:4.1.0-dev" "-p" "frame-support:4.0.0-dev" "-p" "pallet-tips:4.0.0-dev" "-p" "frame-executive:4.0.0-dev" "-p" "sc-rpc-server:4.0.0-dev" "-p" "sp-block-builder:4.0.0-dev" "-p" "generate-bags:4.0.0-dev" "-p" "substrate-frame-rpc-system:4.0.0-dev" "-p" "frame-election-provider-support:4.0.0-dev" "-p" "sc-executor:0.10.0-dev" "-p" "sc-finality-grandpa-rpc:0.10.0-dev" "-p" "sp-externalities:0.10.0-dev" "-p" "sp-consensus-slots:0.10.0-dev" "-p" "substrate-test-utils-derive:0.10.0-dev" "-p" "pallet-staking-reward-fn:4.0.0-dev" "-p" "sc-transaction-pool-api:4.0.0-dev" "-p" "pallet-offences:4.0.0-dev" "-p" "sp-consensus-babe:0.10.0-dev" "-p" "sc-informant:0.10.0-dev" "-p" "pallet-treasury:4.0.0-dev" "-p" "sp-timestamp:4.0.0-dev" "-p" "sp-transaction-pool:4.0.0-dev" "-p" "sc-executor-common:0.10.0-dev" "-p" "beefy-merkle-tree:4.0.0-dev" "-p" "frame-system:4.0.0-dev" "-p" "sp-inherents:4.0.0-dev" "-p" "sp-session:4.0.0-dev" "-p" "sp-database:4.0.0-dev" "-p" "sc-light:4.0.0-dev" "-p" "substrate-test-utils:4.0.0-dev" "-p" "pallet-identity:4.0.0-dev" "-p" "pallet-elections-phragmen:5.0.0-dev" "-p" "pallet-membership:4.0.0-dev" "-p" "pallet-nicks:4.0.0-dev" "-p" "pallet-scheduler:4.0.0-dev" "-p" "pallet-authority-discovery:4.0.0-dev" "-p" "pallet-bags-list-remote-tests:4.0.0-dev" "-p" "pallet-staking:4.0.0-dev" "-p" "sc-chain-spec:4.0.0-dev" "-p" "sc-consensus-uncles:0.10.0-dev" "-p" "sc-executor-wasmi:0.10.0-dev" "-p" "sc-executor-wasmtime:0.10.0-dev" "-p" "pallet-session-benchmarking:4.0.0-dev" "-p" "sc-proposer-metrics:0.10.0-dev" "-p" "sp-arithmetic:4.0.0-dev" "-p" "sp-std:4.0.0-dev" "-p" "frame-system-benchmarking:4.0.0-dev" "-p" "remote-externalities:0.10.0-dev" "-p" "sp-blockchain:4.0.0-dev" "-p" "pallet-babe:4.0.0-dev" "-p" "pallet-authorship:4.0.0-dev" "-p" "pallet-balances:4.0.0-dev" "-p" "frame-try-runtime:0.10.0-dev" "-p" "pallet-timestamp:4.0.0-dev" "-p" "sc-offchain:4.0.0-dev" "-p" "sc-rpc-api:0.10.0-dev" "-p" "sc-finality-grandpa:0.10.0-dev" "-p" "test-runner:0.9.0" "-p" "substrate-test-client:2.0.1" "-p" "substrate-prometheus-endpoint:0.10.0-dev" "-p" "sp-keystore:0.10.0-dev" "-p" "sp-tasks:4.0.0-dev" "-p" "pallet-vesting:4.0.0-dev" "-p" "sp-core:4.0.0-dev" "-p" "pallet-recovery:4.0.0-dev" "-p" "sp-consensus-vrf:0.10.0-dev" "-p" "sc-sync-state-rpc:0.10.0-dev" "-p" "sc-consensus-epochs:0.10.0-dev" "-p" "fork-tree:3.0.0" "-p" "sc-chain-spec-derive:4.0.0-dev" "-p" "pallet-multisig:4.0.0-dev", kill_on_drop: false }", status_code: Some(101), err: " Updating git repository https://github.com/paritytech/substrate\n Updating crates.io index\nerror: no matching package named frame-benchmarking-cli found\nlocation searched: https://github.com/paritytech/substrate?branch=master\nrequired by package polkadot-cli v0.9.12 (/builds/polkadot/cli)\n" }

@ordian
Merge branch 'master' into sandreim/per_subsystem_task_metrics
57a2e32 
* master:
  CI: chore (#3957)
  Companion – Update jsonrpsee to 0.4.1 (#4256)
  Add more XCM tracing (#4211)
  Update dependencies for latest substrate master (#4258)
  Bump mick-jaeger from 0.1.4 to 0.1.6 (#4249)
  Bump dlmalloc from 0.2.2 to 0.2.3 (#4250)
  Bump libc from 0.2.106 to 0.2.107 (#4235)
  Bump paste from 1.0.5 to 1.0.6 (#4244)
  Bump serde_json from 1.0.68 to 1.0.69 (#4236)
@ordian
update Cargo.lock
9cc4620
@ordian
Copy link
Member

ordian commented Nov 11, 2021

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Checks failed for 9cc4620

@ordian
Copy link
Member

ordian commented Nov 11, 2021

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 9e8b26f into master Nov 11, 2021
@paritytech-processbot paritytech-processbot bot deleted the sandreim/per_subsystem_task_metrics branch November 11, 2021 18:53
ordian added a commit that referenced this pull request Nov 12, 2021
@ordian
Merge branch 'master' into 3x8_update_nightly_fix_fmt
6efdbd6 
* master: (71 commits)
  Remove light client companion (#4191)
  Enable full use of pallet-bags-list in westend and kusama runtimes (#4195)
  collator-protocol: do not connect to the next group (#4261)
  Per subsystem CPU usage tracking (#4239)
  CI: chore (#3957)
  Companion – Update jsonrpsee to 0.4.1 (#4256)
  Add more XCM tracing (#4211)
  Update dependencies for latest substrate master (#4258)
  Bump mick-jaeger from 0.1.4 to 0.1.6 (#4249)
  Bump dlmalloc from 0.2.2 to 0.2.3 (#4250)
  Bump libc from 0.2.106 to 0.2.107 (#4235)
  Bump paste from 1.0.5 to 1.0.6 (#4244)
  Bump serde_json from 1.0.68 to 1.0.69 (#4236)
  Update `wasmtime` and related dependencies (companion for Substrate#10149) (#4210)
  update cargo lock to unbreak dep of a dep (#4245)
  Increase maximum chunk size to adjust for small networks. (#4220)
  availability recovery type name clarifications (#4203)
  Update `bridge/` codeowners (#4222)
  fix(staking miner): use `StorageKey` in getStorage (#4231)
  Change path for the tests to master (#4223)
  ...
drahnr pushed a commit that referenced this pull request Nov 15, 2021
@sandreim @ordian @drahnr
Per subsystem CPU usage tracking (#4239)
ab981ac 
* SubsystemContext: add subsystem name str

Signed-off-by: Andrei Sandu <[email protected]>

* Overseer builder proc macro changes

* initilize SubsystemContext name field.
* Add subsystem name in TaskKind::launch_task()

Signed-off-by: Andrei Sandu <[email protected]>

* Update ToOverseer enum

Signed-off-by: Andrei Sandu <[email protected]>

* Assign subsystem names to orphan tasks

Signed-off-by: Andrei Sandu <[email protected]>

* cargo fmt

Signed-off-by: Andrei Sandu <[email protected]>

* SubsystemContext: add subsystem name str

Signed-off-by: Andrei Sandu <[email protected]>

* Overseer builder proc macro changes

* initilize SubsystemContext name field.
* Add subsystem name in TaskKind::launch_task()

Signed-off-by: Andrei Sandu <[email protected]>

* Update ToOverseer enum

Signed-off-by: Andrei Sandu <[email protected]>

* Assign subsystem names to orphan tasks

Signed-off-by: Andrei Sandu <[email protected]>

* cargo fmt

Signed-off-by: Andrei Sandu <[email protected]>

* Rebase changes for new spawn() group param

Signed-off-by: Andrei Sandu <[email protected]>

* Add subsystem constat in JobTrait

Signed-off-by: Andrei Sandu <[email protected]>

* Add subsystem string

Signed-off-by: Andrei Sandu <[email protected]>

* Fix tests

Signed-off-by: Andrei Sandu <[email protected]>

* Fix spawn() calls

Signed-off-by: Andrei Sandu <[email protected]>

* cargo fmt

Signed-off-by: Andrei Sandu <[email protected]>

* Fix

Signed-off-by: Andrei Sandu <[email protected]>

* Fix tests

Signed-off-by: Andrei Sandu <[email protected]>

* fix

Signed-off-by: Andrei Sandu <[email protected]>

* Fix more tests

Signed-off-by: Andrei Sandu <[email protected]>

* Address PR review feedback #1

Signed-off-by: Andrei Sandu <[email protected]>

* Address PR review round 2

Signed-off-by: Andrei Sandu <[email protected]>

* Fixes
- remove JobTrait::Subsystem
- fix tests

Signed-off-by: Andrei Sandu <[email protected]>

* update Cargo.lock

Co-authored-by: Andronik Ordian <[email protected]>
@github-actions github-actions bot mentioned this pull request Dec 2, 2021
Closed
@ghzlatarev ghzlatarev mentioned this pull request Dec 4, 2021
Merged
23 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@drahnr drahnr drahnr approved these changes

@ordian ordian ordian approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sandreim @ordian @drahnr