You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "juncevich (via GitHub)" <gi...@apache.org> on 2025/01/16 08:59:01 UTC
[PR] HDDS-9377. Call MutableStat.add(long x) method as a runnable task in a separate service executor [ozone]
juncevich opened a new pull request, #7708:
URL: https://github.com/apache/ozone/pull/7708
## What changes were proposed in this pull request?
Change synchronized metrics collect methods to read/write lock implementation.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9377
## How was this patch tested?
I wrote the JUnit test (I'm not sure I should add this test to this PR) to compare performance metric realization.
```
@RepeatedTest(100)
void testMyMetrics() throws InterruptedException {
OzoneMutableRate rate = new OzoneMutableRate("Test name", "Test description", true);
final MetricsRecordBuilder builder = getMetricsRecordBuilder();
ExecutorService executor = Executors.newFixedThreadPool(10);
for (int i = 0; i < 100; i++) {
int finalI = i;
executor.submit(() -> {
rate.add(finalI);
if (finalI % 100 == 0) {
rate.snapshot(getMetricsRecordBuilder());
}
});
}
executor.awaitTermination(1, TimeUnit.SECONDS);
rate.snapshot(builder);
assertEquals(100, builder.metrics().get(0).value().intValue());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7708:
URL: https://github.com/apache/ozone/pull/7708#discussion_r1949867014
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/metrics/OzoneMutableStat.java:
##########
@@ -0,0 +1,251 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.metrics;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.MetricsRecordBuilder;
+import org.apache.hadoop.metrics2.lib.MutableStat;
+import org.apache.hadoop.metrics2.util.SampleStat;
+import org.apache.hadoop.util.Time;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * A convenient mutable metric for throughput measurement.
+ * (non-synchronized version of org.apache.hadoop.metrics2.lib.MutableStat)
+ */
+public class OzoneMutableStat extends MutableStat {
Review Comment:
Is it supposed to extend `MutableStat`? This duplicates all member variables. Can we copy all necessary classes without subclassing?
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/ContainerClientMetrics.java:
##########
@@ -56,27 +56,27 @@ public final class ContainerClientMetrics {
private MutableCounterLong totalWriteChunkBytes;
@Metric
- private MutableRate hsyncSynchronizedWorkNs;
+ private OzoneMutableRate hsyncSynchronizedWorkNs;
Review Comment:
If possible, we should keep the original class names. Package name should be enough to distinguish between Hadoop and Ozone class. This would help reduce code change mostly to imports.
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/metric/SampleQuantilesTest.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.metric;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.ozone.metrics.OzoneSampleQuantiles;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests for SampleQuantiles.
+ */
+class SampleQuantilesTest {
Review Comment:
Tests named `*Test` are not executed:
https://github.com/apache/ozone/blob/6f82e7b77c6463d3f9d2c9c967dc988b6e208066/pom.xml#L1844
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2756923754
@adoroszlai Hi, Attilla! Can you tell me what I should do with this draft? I fixed your review notices and refreshed this branch from the master. Should I convert this draft to a pull request? Or should I close this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2756935868
Thanks @juncevich for updating the patch. I will take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2635984457
> @juncevich I have a few questions, what is the desired impact of this code change? Do you see a performance difference? How much is it? Answers to those questions will help us decide if it is ok to duplicate code but fix it to gain benefits. In the linked jira you do describe the problem but do not mention how much this PR will help solve the issue. Also, internal improvements sometimes do not lead to externally visible improvements, any details on how it helped speed up the test you'll deviced?
@kerneltime The desired impact is to get rid of the locks in metrics collecting and improve writing speed. Without a fix, it was a 3000-3200 writing operation (CreateFile and CommitKey). After fixing it was 3600-3700.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2635034797
I looked at communication in Hadoop(mailing lists) and discussion and realization can take a long time )
I think the best way is not to depend on Hadoop libraries and change the metric library to another instead of improving the Hadoop metric library.
I offer to close this PR and create one task to change the metric library and (I'm not sure) another to create an Ozone metric API which can be implemented by any metric library.
What's your opinion, @adoroszlai ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2774876380
Thanks again @juncevich for the patch. I suggest splitting it:
1. Copy Hadoop's complete metrics library as is. No changes at all. Will need to add exclusions for checkstyle, maybe findbugs and pmd, too.
2. Fix checkstyle and other violations previously suppressed. Remove exclusions.
3. Move to Ozone-specific package and change existing Ozone code to use it from the new package.
4. Make performance improvements (RW lock instead of synchronization, etc.).
Benefits:
- scope of each change is well-defined, easier to review
- first 3 changes can be reviewed by any committer, no performance-related experience necessary
- easier to share improvements from (4) with the Hadoop project
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on code in PR #7708:
URL: https://github.com/apache/ozone/pull/7708#discussion_r1951597549
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/metrics/OzoneMutableStat.java:
##########
@@ -0,0 +1,251 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.metrics;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.metrics2.MetricsInfo;
+import org.apache.hadoop.metrics2.MetricsRecordBuilder;
+import org.apache.hadoop.metrics2.lib.MutableStat;
+import org.apache.hadoop.metrics2.util.SampleStat;
+import org.apache.hadoop.util.Time;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static org.apache.hadoop.metrics2.lib.Interns.info;
+
+/**
+ * A convenient mutable metric for throughput measurement.
+ * (non-synchronized version of org.apache.hadoop.metrics2.lib.MutableStat)
+ */
+public class OzoneMutableStat extends MutableStat {
Review Comment:
All metrics should extend MutableMetric. I will change extending MutableStat to MutableMetric.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2633132555
> Thanks @juncevich for the patch. It seems this removes synchronization in some individual metrics, but not in `MetricsSytemImpl` itself. (see also [HDDS-12193](https://issues.apache.org/jira/browse/HDDS-12193))
>
> My 2 cents: I don't think we should reimplement Hadoop metrics system in Ozone with tweaks. Either improve it in Hadoop to benefit the ecosystem, or use a better existing metrics library in Ozone.
Thanks for the notices. Could you tell me more about the `better existing metrics library in Ozone`? Is it an existing library in Ozone, or have you offered to change the metrics library from Hadoop to another metric system?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2635342600
@juncevich I have a few questions, what is the desired impact of this code change? Do you see a performance difference? How much is it? Answers to those questions will help us decide if it is ok to duplicate code but fix it to gain benefits. In the linked jira you do describe the problem but do not mention how much this PR will help solve the issue. Also, internal improvements sometimes do not lead to externally visible improvements, any details on how it helped speed up the test you'll deviced?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai closed pull request #7708: HDDS-9377. Implement some metrics with RW-lock instead of synchronization
URL: https://github.com/apache/ozone/pull/7708
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2785762681
To be split.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2647561744
> I think we need a dual strategy
I agree.
Please also take a look at Ratis' metrics API, for which Ozone already uses the Dropwizard implementation.
- https://github.com/apache/ratis/tree/master/ratis-metrics-api/src/main/java/org/apache/ratis/metrics
- https://github.com/apache/ratis/tree/master/ratis-metrics-default/src/main/java/org/apache/ratis/metrics/impl
- https://github.com/apache/ratis/tree/master/ratis-metrics-dropwizard3/src/main/java/org/apache/ratis/metrics/dropwizard3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2645513610
> I looked at communication in Hadoop(mailing lists) and discussion and realization can take a long time ) I think the best way is not to depend on Hadoop libraries and change the metric library to another instead of improving the Hadoop metric library. I offer to close this PR and create one task to change the metric library and (I'm not sure) another to create an Ozone metric API which can be implemented by any metric library. What's your opinion, @adoroszlai ?
If we are to replace the metric implementation, I would go with something like [dropwizard](https://metrics.dropwizard.io/4.2.0/) or another existing package to reuse after comparing performance.
> @kerneltime The desired impact is to get rid of the locks in metrics collecting and improve writing speed. Without a fix, it was a 3000-3200 writing operation (CreateFile or CommitKey). After fixing it was 3600-3700. Improvement of more than 12%.
This is a decent improvement, @adoroszlai should we initiate an upstream discussion with Hadoop while we see if we can merge this change in. I think we need a dual strategy
1. Evaluate a replacement for Hadoop metrics
2. Start a PR upstream with Hadoop while we move Ozone to use the more performant solution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2633192842
> > Could you tell me more about the `better existing metrics library in Ozone`? Is it an existing library in Ozone, or have you offered to change the metrics library from Hadoop to another metric system?
>
> Not "existing in Ozone", I meant "existing somewhere else, which we could start using in Ozone".
>
> 1. copy Hadoop's metrics system and improve
> 2. improve in Hadoop
> 3. replace
>
> I consider (1) to be the worst option, both for Ozone and globally.
I think (2) is the best option, but it takes the longest. I will ask about not synchronizing metrics in the Hadoop project. And will return with the answer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2633168885
> Could you tell me more about the `better existing metrics library in Ozone`? Is it an existing library in Ozone, or have you offered to change the metrics library from Hadoop to another metric system?
Not "existing in Ozone", I meant "existing somewhere else, which we could start using in Ozone".
1. copy Hadoop's metrics system and improve
2. improve in Hadoop
3. replace
I consider (1) to be the worst option, both for Ozone and globally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on code in PR #7708:
URL: https://github.com/apache/ozone/pull/7708#discussion_r1951603577
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/metric/SampleQuantilesTest.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.metric;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.hadoop.metrics2.util.Quantile;
+import org.apache.hadoop.ozone.metrics.OzoneSampleQuantiles;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests for SampleQuantiles.
+ */
+class SampleQuantilesTest {
Review Comment:
Thanks. Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement some metrics with RW-lock instead of synchronization [ozone]
Posted by "juncevich (via GitHub)" <gi...@apache.org>.
juncevich commented on code in PR #7708:
URL: https://github.com/apache/ozone/pull/7708#discussion_r1951598430
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/ContainerClientMetrics.java:
##########
@@ -56,27 +56,27 @@ public final class ContainerClientMetrics {
private MutableCounterLong totalWriteChunkBytes;
@Metric
- private MutableRate hsyncSynchronizedWorkNs;
+ private OzoneMutableRate hsyncSynchronizedWorkNs;
Review Comment:
Good idea! Will fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9377. Implement metrics system with RW-lock instead of synchronization [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7708:
URL: https://github.com/apache/ozone/pull/7708#issuecomment-2633113616
Thanks @juncevich for the patch. It seems this removes synchronization in some individual metrics, but not in `MetricsSytemImpl` itself. (see also HDDS-12193)
My 2 cents: I don't think we should reimplement Hadoop metrics system in Ozone with tweaks. Either improve it in Hadoop to benefit the ecosystem, or use a better existing metrics library in Ozone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org