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

[SPARK-39944][BUILD] Upgrade dropwizard metrics to 4.2.10 #37372

Closed
wants to merge 3 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Aug 2, 2022

What changes were proposed in this pull request?

This pr upgrade dropwizard metrics from 4.2.7 to 4.2.10 and changes the links http://metrics.dropwizard.io/4.2.0 in docs/monitoring.md to https://metrics.dropwizard.io/4.2.0

Why are the changes needed?

There are 3 versions after 4.2.7, the release notes as follows:

The new version brings a new API for more type safe of registering gauges(dropwizard/metrics#2642)

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

@github-actions github-actions bot added the BUILD label Aug 2, 2022
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you, @LuciferYang . Please update the doc together.

$ git grep 4.2.0 docs/monitoring.md
docs/monitoring.md:Executor memory metrics are also exposed via the Spark metrics system based on the [Dropwizard metrics library](http://metrics.dropwizard.io/4.2.0).
docs/monitoring.md:[Dropwizard Metrics Library](http://metrics.dropwizard.io/4.2.0).
docs/monitoring.md:see [Dropwizard library documentation for details](https://metrics.dropwizard.io/4.2.0/getting-started.html).
docs/monitoring.md:  [Dropwizard/Codahale Metric Sets for JVM instrumentation](https://metrics.dropwizard.io/4.2.0/manual/jvm.html)

@LuciferYang
Copy link
Contributor Author

@dongjoon-hyun There should be no new page to update. https://metrics.dropwizard.io/4.2.0/ seems to be the latest, there is no https://metrics.dropwizard.io/4.2.10/ similar pages exist. If it's my problem, please correct me.

In addition, do we need to change http to https? For example:

docs/monitoring.md:Executor memory metrics are also exposed via the Spark metrics system based on the [Dropwizard metrics library](http://metrics.dropwizard.io/4.2.0).

@dongjoon-hyun
Copy link
Member

Oh, got it. It's too bad for us because we cannot have a matched doc inevitably. For https, yes, it will be better to have https if it works.

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Aug 2, 2022

Oh, got it. It's too bad for us because we cannot have a matched doc inevitably. For https, yes, it will be better to have https if it works.

OK, will update this later

@github-actions github-actions bot added the DOCS label Aug 2, 2022
@LuciferYang
Copy link
Contributor Author

Oh, got it. It's too bad for us because we cannot have a matched doc inevitably. For https, yes, it will be better to have https if it works.

OK, will update this later

done

@srowen
Copy link
Member

srowen commented Aug 2, 2022

I think the error is unrelated, but can we trigger tests again here?

@LuciferYang
Copy link
Contributor Author

image

Sparkr test failed, and it should not pass all now, master has the same problem

https://github.com/apache/spark/runs/7643052356?check_suite_focus=true

@dongjoon-hyun
Copy link
Member

Yes, #37399 is trying to fix the SparkR failure .
Let me merge this. Thank you, @LuciferYang , @HyukjinKwon , @srowen .

@LuciferYang
Copy link
Contributor Author

thanks @dongjoon-hyun @HyukjinKwon @srowen

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