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

STORM-3781 switch recv-iconnection to v2 metric api #3402

Closed
wants to merge 1 commit into from

Conversation

agresch
Copy link
Contributor

@agresch agresch commented Jul 2, 2021

What is the purpose of the change

Updates metrics to V2 API.

How was the change tested

Ran storm-client unit tests. Validated metrics log for word count topology.

for (Map.Entry<String, AtomicLong> ent : byteCounts.entrySet()) {
AtomicLong count = ent.getValue();
if (count.get() > 0) {
counts.put(ent.getKey(), count.getAndSet(0L));
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this won't support multiple reporters?
In the case of multiple reporters being configured, this getAndSet will return differently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a good way to support that with the V2 interface. Possibly we could provide hooks to allow the metrics to know what reporters exist and when they report. Then they could defer resetting until all reporters have reported. But even then there could be frequency differences between multiple reporters.

We could leave these as V1 metrics if that is preferred. I am not sure how often these are used.

if (metricRegistry != null) {
Gauge<Map<String, Integer>> enqueued = new Gauge<Map<String, Integer>>() {
@Override
public Map<String, Integer> getValue() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it support having multiple reporters?

@typercode
Copy link

I am a newcomer to Storm, I have a question to ask, how did STORM-3781 come from?

@agresch
Copy link
Contributor Author

agresch commented Jan 3, 2022

The recv-iconnection metrics are using a deprecated (v1) metrics API. This JIRA is to update them to the newer v2 API.

@agresch
Copy link
Contributor Author

agresch commented Mar 28, 2022

closing for now

@agresch agresch closed this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants