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

Update throttling counters to use generic throttling-msecs metric and dedicated namespace #19632

Closed
damccorm opened this issue Jun 4, 2022 · 4 comments · Fixed by #31924
Closed

Comments

@damccorm
Copy link
Contributor

damccorm commented Jun 4, 2022

Update throttling counters to use generic throttling-msecs metric and dedicated namespace. Currently, throttle time metric is saved in a user counter that a namespace is a class name and a counter name is "cumulativeThrottlingSeconds".

Imported from Jira BEAM-7863. Original Jira may contain additional context.
Reported by: heejong.

@Naireen
Copy link
Contributor

Naireen commented May 24, 2023

There are some throttling counters that don't follow that convention, eg the BQ streaming inserts throttling counter: https://github.com/apache/beam/blob/d91ea8e44918a7ef10ac8b33001cddab61bcedc5/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryServicesImpl.java#LL561C1-L562C71

How do you envision the end state to look? If there is a single metric for the throttling counter, should all IO's update only that counter?

@damccorm
Copy link
Contributor Author

@ihji filed this one originally, I don't have a ton of context. @johnjcasey may have thoughts

@johnjcasey
Copy link
Contributor

I don't have context on this. This looks like something that requires a bit of a doc to find the vision

@lostluck
Copy link
Contributor

lostluck commented Jul 25, 2023

Dataflow side just discussed this a bit, so forwarding some key points:

Beam Metrics over the FnAPI are always qualified by Transform, Namespace, Name, and additionally by Bundle. That last one is ephemeral, and used to get precise counters WRT bundle retries.

So in that respect, a runner can already always "know" which transform a counter comes from, and then act accordingly. But for the most part, there's no reason to distinguish between transforms that are throttling. The key part then is deciding officially on a common "beam" namespace (I'd suggest beam or beam:standard:counter, over something javalike org.apache.beam.standard.count) (I'll go with beam for now) and a name for the counter itself, which might as well be throttling-msecs since there are several IOs already doing that.

So the hard part of this (aside from a dev list community discussion of the proposal) would be clearly documenting that the beam namespace has certain reserved counter names, that runners may take special note of for whatever they need. Throttling is one case here. But even things like rpc-count, rpc-retries, etc etc... And this should be documented clearly in the "IO" guide, and any SDK building guide on the beam website.

It's entirely possible that the community decision could be the name "throttling-msecs" is special by itself, and runners should pay attention to any and all of them for throttling decisions. But TBH I think that's only useful if a single DoFn has multiple RPC services to wait on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants