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.

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

[Go SDK] Add more info to Worker Status API #21776

Merged
merged 7 commits into from
Jun 14, 2022

Conversation

riteshghorse
Copy link
Contributor

@riteshghorse riteshghorse commented Jun 9, 2022

Adds Memory Usage information, Cache Stats, and Active Process Bundle States to worker status API added in #16957

Example Output (on Dataflow):

============Memory Usage============ 
heap in-use-spans/allocated/total/max = 5/4/15/7 MB stack in-use-spans/max = 0/0 MB GC-CPU percentage = 0.00 % Last GC time: 2022-06-10 16:34:35.629059975 +0000 UTC Next GC: 6 MB 
============Active Process Bundle States============ 
Bundle ID: process_bundle-369391211043651523-0 Bundle State: PTransform ID: CountWords/wordcount.extractFn-ptransform-40 Current state: Process Bundle All Bundle Process States: Bundle ID: CountWords/stats.Count/stats.keyedCountFn-ptransform-41 Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Bundle ID: CountWords/wordcount.extractFn-ptransform-40 Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 9m57.8s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 9m57.8s Bundle ID: beam.createFn-ptransform-39 Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Bundle ID: CountWords/stats.Count/stats.SumPerKey/CombinePerKey/CoGBK+CountWords/stats.Count/stats.SumPerKey/CombinePerKey/stats.sumIntFn/Partial-ptransform-42 Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s Execution State: State: Start Bundle IsProcessing: false Total time: 0s
 ============Cache Stats============ 
State Cache: {Hits:0 Misses:0 Evictions:0 InUseEvictions:0 ReStreamErrors:0} 
============Goroutine Dump============ 
goroutine profile: total 24 4 @ 0x438e96 0x431897 0x463909 0x49f1d2 0x4a053a 0x4a0528 0x6d2349 0x6e3d05 0x524874

See the bottom of https://docs.google.com/document/d/1dMTD5_sKdzLcnoe0ZsQU5Wf9q11uliyYgFnnOZQDzuI/ for more details.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@riteshghorse riteshghorse marked this pull request as ready for review June 9, 2022 19:57
@asf-ci
Copy link

asf-ci commented Jun 9, 2022

Can one of the admins verify this patch?

1 similar comment
@asf-ci
Copy link

asf-ci commented Jun 9, 2022

Can one of the admins verify this patch?

@github-actions github-actions bot added the go label Jun 9, 2022
@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #21776 (ac6fc70) into master (7bf822a) will decrease coverage by 0.02%.
The diff coverage is 38.70%.

@@            Coverage Diff             @@
##           master   #21776      +/-   ##
==========================================
- Coverage   74.09%   74.07%   -0.03%     
==========================================
  Files         698      698              
  Lines       92503    92574      +71     
==========================================
+ Hits        68544    68577      +33     
- Misses      22704    22742      +38     
  Partials     1255     1255              
Flag Coverage Δ
go 50.82% <38.70%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdks/go/pkg/beam/core/metrics/store.go 10.63% <0.00%> (-4.99%) ⬇️
sdks/go/pkg/beam/core/runtime/harness/harness.go 10.20% <0.00%> (-0.09%) ⬇️
...beam/core/runtime/harness/statecache/statecache.go 74.65% <0.00%> (-2.11%) ⬇️
.../go/pkg/beam/core/runtime/harness/worker_status.go 72.36% <94.73%> (+21.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bf822a...ac6fc70. Read the comment docs.

@github-actions github-actions bot added the docker label Jun 9, 2022
@riteshghorse
Copy link
Contributor Author

Run GoPortable PostCommit

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2022

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

sdks/go/pkg/beam/core/runtime/harness/worker_status.go Outdated Show resolved Hide resolved
sdks/go/pkg/beam/core/runtime/harness/worker_status.go Outdated Show resolved Hide resolved
sdks/go/pkg/beam/core/runtime/harness/worker_status.go Outdated Show resolved Hide resolved
sdks/go/pkg/beam/core/runtime/harness/worker_status.go Outdated Show resolved Hide resolved
@@ -18,15 +18,13 @@ package wordcount

import (
"context"
"regexp"
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this change. At best, we mostly want to remove the blank line between "strings" and "fmt".

sdks/go/pkg/beam/core/runtime/harness/worker_status.go Outdated Show resolved Hide resolved
@@ -37,26 +38,15 @@ import (
"google.golang.org/protobuf/types/known/durationpb"
)

// StatusAddress is a type of status endpoint address as an optional argument to harness.Main().
type StatusAddress string

// TODO(herohde) 2/8/2017: for now, assume we stage a full binary (not a plugin).

// Main is the main entrypoint for the Go harness. It runs at "runtime" -- not
// "pipeline-construction time" -- on each worker. It is a FnAPI client and
// ultimately responsible for correctly executing user code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add documentation around "expected" Environment variables. TBH, I don't mind the options approach to pass into main, but the fetching form the Env vars would then happen in the init.go.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2022

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @jrmccluskey for label go.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@lostluck
Copy link
Contributor

lostluck commented Jun 9, 2022

Run Go Postcommit

@lostluck lostluck self-requested a review June 9, 2022 23:46
@riteshghorse
Copy link
Contributor Author

Made the changes, PTAL

@riteshghorse
Copy link
Contributor Author

Run Go PostCommit

@lostluck lostluck merged commit 091c05c into apache:master Jun 14, 2022
bullet03 pushed a commit to akvelon/beam that referenced this pull request Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants