You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/04/20 13:28:31 UTC
[GitHub] [pulsar] Shoothzj opened a new pull request, #15238: use netty PlatformDependent.estimateMaxDirectMemory
Shoothzj opened a new pull request, #15238:
URL: https://github.com/apache/pulsar/pull/15238
### Motivation
- `PlatformDependent.maxDirectMemory()` can be inaccurate if `io.netty.maxDirectMemory` are setted
- Bookkeeper's `DirectMemoryUtils` is not worked within some jdk releases.
In netty 4.1.75, they introduced a new method `PlatformDependent.estimateMaxDirectMemory` to help users get maxDirectMemory. Since netty's this method works well on many jdk releases, use this to replace below two.
PS: `DirectMemoryUtils` has been removed from bookkeeper in https://github.com/apache/bookkeeper/pull/2989
### Modifications
- use `PlatformDependent.estimateMaxDirectMemory` instead
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126587693
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126862488
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126881541
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126693766
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1125669466
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126884858
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126621107
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126596101
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1120371515
@michaeljmarshall Please take a look again. @codelipenghui @eolivelli @merlimat PTAL
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126910581
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1121471566
> @michaeljmarshall My bad The main reason I want to replace is that `maxDirectMemory()` can be changed by system property `io.netty.maxDirectMemory`, which will may lead to wrong result. To avoid performance regression, I think we can put it in a common class, and others use the result. What do you think?
Is it possible that it is a feature, not a bug, that we expose `io.netty.maxDirectMemory` to end users? If a user configures `io.netty.maxDirectMemory` poorly, I wouldn't consider that a performance regression.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1124463178
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126713526
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126684572
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126700837
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126880775
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1104609391
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126651713
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126869279
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126890029
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1123359034
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a diff in pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on code in PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#discussion_r869172301
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/DirectMemoryUtils.java:
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.pulsar.common.util;
+
+// CHECKSTYLE.OFF: IllegalImport
+import io.netty.util.internal.PlatformDependent;
+// CHECKSTYLE.ON: IllegalImport
+
+public class DirectMemoryUtils {
+
+ /**
+ * PlatformDependent.maxDirectMemory can be inaccurate if java property `io.netty.maxDirectMemory` are setted.
+ * Cache the result in this field.
+ */
+ public static final long JVM_MAX_DIRECT_MEMORY = PlatformDependent.estimateMaxDirectMemory();
Review Comment:
@lhotari 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#discussion_r869155597
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/DirectMemoryUtils.java:
##########
@@ -0,0 +1,33 @@
+/**
+ * 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.pulsar.common.util;
+
+// CHECKSTYLE.OFF: IllegalImport
+import io.netty.util.internal.PlatformDependent;
+// CHECKSTYLE.ON: IllegalImport
+
+public class DirectMemoryUtils {
+
+ /**
+ * PlatformDependent.maxDirectMemory can be inaccurate if java property `io.netty.maxDirectMemory` are setted.
+ * Cache the result in this field.
+ */
+ public static final long JVM_MAX_DIRECT_MEMORY = PlatformDependent.estimateMaxDirectMemory();
Review Comment:
It's not a good idea to make this a field. In general, a method is better since it can hide the implementation details in a better way. You can have the value cached using a private field if it's necessary to cache the result.
I would suggest switching to use a method instead of exposing a field.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126639647
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on code in PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#discussion_r854747766
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGenerator.java:
##########
@@ -70,7 +70,7 @@ public double get() {
Gauge.build("jvm_memory_direct_bytes_max", "-").create().setChild(new Child() {
@Override
public double get() {
- return io.netty.util.internal.PlatformDependent.maxDirectMemory();
+ return io.netty.util.internal.PlatformDependent.estimateMaxDirectMemory();
Review Comment:
It looks like the javadoc for this method recommends using `#maxDirectMemory()`:
```java
/**
* Compute an estimate of the maximum amount of direct memory available to this JVM.
* <p>
* The computation is not cached, so you probably want to use {@link #maxDirectMemory()} instead.
* <p>
* This will produce debug log output when called.
*
* @return The estimated max direct memory, in bytes.
*/
public static long estimateMaxDirectMemory()
```
https://github.com/netty/netty/blob/0d653f08cd108a3ba6cf76d9fc052b8141ea3196/common/src/main/java/io/netty/util/internal/PlatformDependent.java#L1154-L1163
I am concerned that the cost of this alternative method. Are you able to provide documentation on how this method is better? Alternatively, is there a reason we can't cache the result in this class and the other references that will be called many times?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1105110631
@michaeljmarshall My bad
The main reason I want to replace is that `maxDirectMemory()` can be changed by system property `io.netty.maxDirectMemory`, which will may lead to wrong result.
To avoid performance regression, I think we can put it in a common class, and others use the result.
What do you think?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1124414787
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1123116386
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1122323717
@lhotari PTAL again. @codelipenghui @eolivelli @merlimat PTAL
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on a diff in pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on code in PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#discussion_r869368709
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LoadManagerShared.java:
##########
@@ -225,7 +225,7 @@ public static SystemResourceUsage getSystemResourceUsage(final BrokerHostUsage b
// Collect JVM direct memory
systemResourceUsage.setDirectMemory(new ResourceUsage((double) (getJvmDirectMemoryUsed() / MIBI),
- (double) (io.netty.util.internal.PlatformDependent.maxDirectMemory() / MIBI)));
+ (double) (io.netty.util.internal.PlatformDependent.estimateMaxDirectMemory() / MIBI)));
Review Comment:
@lhotari Sorry. It's a miss. It have been 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj merged pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj merged PR #15238:
URL: https://github.com/apache/pulsar/pull/15238
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1121676817
@michaeljmarshall There are two scenes using this method
- metrics, using this method to produce maxDirectMemory metrics, `estimateMaxDirectMemory` is more proper.
- Serverside config `maxMessagePublishBufferSizeInMB`. This param's default value is 1/2 of direct memory. `io.netty.maxDirectMemory` isn't on our docs. They can reconfigure it by `maxMessagePublishBufferSizeInMB`.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] coderzc commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1122341091
LGTM
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#discussion_r869299895
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LoadManagerShared.java:
##########
@@ -225,7 +225,7 @@ public static SystemResourceUsage getSystemResourceUsage(final BrokerHostUsage b
// Collect JVM direct memory
systemResourceUsage.setDirectMemory(new ResourceUsage((double) (getJvmDirectMemoryUsed() / MIBI),
- (double) (io.netty.util.internal.PlatformDependent.maxDirectMemory() / MIBI)));
+ (double) (io.netty.util.internal.PlatformDependent.estimateMaxDirectMemory() / MIBI)));
Review Comment:
Is it intentional to keep on using PlatformDependent class here?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126606252
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126627701
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126613808
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on code in PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#discussion_r854747766
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGenerator.java:
##########
@@ -70,7 +70,7 @@ public double get() {
Gauge.build("jvm_memory_direct_bytes_max", "-").create().setChild(new Child() {
@Override
public double get() {
- return io.netty.util.internal.PlatformDependent.maxDirectMemory();
+ return io.netty.util.internal.PlatformDependent.estimateMaxDirectMemory();
Review Comment:
It looks like the javadoc for this method recommends using `#maxDirectMemory()`:
https://github.com/netty/netty/blob/0d653f08cd108a3ba6cf76d9fc052b8141ea3196/common/src/main/java/io/netty/util/internal/PlatformDependent.java#L1154-L1163
I am concerned that the cost of this alternative method. Are you able to provide documentation on how this method is better? Alternatively, is there a reason we can't cache the result in this class and the other references that will be called many times?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] HQebupt commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
HQebupt commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1104052192
LGTM :)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126634452
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126664507
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126675742
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126709121
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1126920668
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1124484304
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Shoothzj commented on pull request #15238: use netty PlatformDependent.estimateMaxDirectMemory
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #15238:
URL: https://github.com/apache/pulsar/pull/15238#issuecomment-1124526638
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org