Skip to content

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

FMX pushed a commit to apache/celeborn that referenced this pull request Jun 4, 2024
### What changes were proposed in this pull request?

Bump Dropwizard version from 3.2.6 to 4.2.25. Meanwhile, introduce `metrics_jvm_thread_peak_count_Value` and `metrics_jvm_thread_total_started_count_Value` in `celeborn-jvm-dashboard.json`.

### Why are the changes needed?

Dropwizard metrics has released v4.2.25 including some bugfixes and improvements including:

* [JVM] Fix maximum/total memory calculation: dropwizard/metrics#3125
* [Thread] Add peak and total started thread count to `ThreadStatesGaugeSet`: dropwizard/metrics#1601

Meanwhile, Ratis version has upgraded to 3.0.1 which has no compatibility problem with Dropwizard 4.2.25.

Backport:

- apache/spark#26332
- apache/spark#29426
- apache/spark#37372

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Manual test.

Closes #2540 from SteNicholas/CELEBORN-1389.

Authored-by: SteNicholas <[email protected]>
Signed-off-by: mingji <[email protected]>
FMX pushed a commit to apache/celeborn that referenced this pull request Jun 4, 2024
### What changes were proposed in this pull request?

Bump Dropwizard version from 3.2.6 to 4.2.25. Meanwhile, introduce `metrics_jvm_thread_peak_count_Value` and `metrics_jvm_thread_total_started_count_Value` in `celeborn-jvm-dashboard.json`.

### Why are the changes needed?

Dropwizard metrics has released v4.2.25 including some bugfixes and improvements including:

* [JVM] Fix maximum/total memory calculation: dropwizard/metrics#3125
* [Thread] Add peak and total started thread count to `ThreadStatesGaugeSet`: dropwizard/metrics#1601

Meanwhile, Ratis version has upgraded to 3.0.1 which has no compatibility problem with Dropwizard 4.2.25.

Backport:

- apache/spark#26332
- apache/spark#29426
- apache/spark#37372

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Manual test.

Closes #2540 from SteNicholas/CELEBORN-1389.

Authored-by: SteNicholas <[email protected]>
Signed-off-by: mingji <[email protected]>
(cherry picked from commit 4fc42d7)
Signed-off-by: mingji <[email protected]>
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.

4 participants