You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/03/06 10:05:15 UTC
[GitHub] [activemq-artemis] atris opened a new pull request #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
atris opened a new pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris opened a new pull request #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris opened a new pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596143770
Actually seems since 2999 was merged https://travis-ci.org/apache/activemq-artemis/builds/658977370?utm_source=github_status&utm_medium=notification
there seems to been a number of merges very close together as such could be any that were merged before 2999 as unfortunately as merges so close together previous builds cancelled for next.
merges between last success build and first build failed, as such these are the smoking guns
2973, 2992, 2993, 2997, 2999
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596142749
@jbertram I'm getting test failures in CLI tests locally.
Failed tests:
MessageSerializerTest.testTextMessageImportExport:177->checkSentMessages:104->CliTestBase.consumeMessages:147 null
Tests in error:
MessageSerializerTest.testMapMessageImportExport:227 » InvalidDestination AMQ2...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r390412940
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2996,6 +2996,7 @@ private void registerMeters() {
builder.register(BrokerMetricNames.CONNECTION_COUNT, this, metrics -> Double.valueOf(getConnectionCount()), ActiveMQServerControl.CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.TOTAL_CONNECTION_COUNT, this, metrics -> Double.valueOf(getTotalConnectionCount()), ActiveMQServerControl.TOTAL_CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.ADDRESS_MEMORY_USAGE, this, metrics -> Double.valueOf(getPagingManager().getGlobalSize()), ActiveMQServerControl.ADDRESS_MEMORY_USAGE_DESCRIPTION);
+ builder.register(BrokerMetricNames.DISK_STORE_USAGE, this, metrics -> Double.valueOf(getPagingManager().getDiskTotalSpace() - getPagingManager().getDiskUsableSpace()), ActiveMQServerControl.DISK_STORE_USAGE_DESCRIPTION);
Review comment:
Since address memory usage is not exposed as a metric here, I think it warrants a separate PR (to add both percentage types to exposed metrics). I will follow up with a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596142749
@jbertram I'm getting test failures in CLI tests locally, when doing mvn clean install -Pfast-tests -Pextra-tests
Failed tests:
MessageSerializerTest.testTextMessageImportExport:177->checkSentMessages:104->CliTestBase.consumeMessages:147 null
Tests in error:
MessageSerializerTest.testMapMessageImportExport:227 » InvalidDestination AMQ2...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-602205807
@jbertram @clebertsuconic @criew Fixed, please see and let me know your comments
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-597434612
@clebertsuconic I believe I did that? (The PR is rebased on top of your latest commit to master, and is squashed to a single commit. What did I miss?)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r396095069
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##########
@@ -707,6 +707,24 @@ public long getAddressMemoryUsage() {
}
}
+ @Override
+ public long getDiskStoreUsed() {
Review comment:
Thanks for validating!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-602621213
How did you come up with these formulas? did they come from the JIRA? or you made up them?
as an user I would prefer seeing how much space I have left.. and what's the percentage of the disk that it's used
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596143333
checking build histories of master on travis, seems as soon as PR #3003 was merged, the build starting failing with the issue, as such does look like it is related to PR #3003
here is the failed build on master straight after 3003 was merged
https://travis-ci.org/apache/activemq-artemis/builds/658977926?utm_source=github_status&utm_medium=notification
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-597222429
Done, please see.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-598333666
Have you sorted better naming so its clearer? See inline comments by clebert and myself
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595817943
can y ou run the test locally and let us know what you see?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595817775
thjere's a test failure here:
MessageSerializerTest.testObjectMessageImportExport:202->CliTestBase.consumeMessages:147 null
Are you sure it's not related? I will have to 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull
request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r392376786
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##########
@@ -707,6 +707,24 @@ public long getAddressMemoryUsage() {
}
}
+ @Override
+ public long getDiskStoreUsed() {
Review comment:
I'm a bit confused between the metric and the math here...
disk space - disk-usable? what that means!?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595791692
the PR test is failing.. can you check why it failed? running tests...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull
request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r391205527
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2996,6 +2996,7 @@ private void registerMeters() {
builder.register(BrokerMetricNames.CONNECTION_COUNT, this, metrics -> Double.valueOf(getConnectionCount()), ActiveMQServerControl.CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.TOTAL_CONNECTION_COUNT, this, metrics -> Double.valueOf(getTotalConnectionCount()), ActiveMQServerControl.TOTAL_CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.ADDRESS_MEMORY_USAGE, this, metrics -> Double.valueOf(getPagingManager().getGlobalSize()), ActiveMQServerControl.ADDRESS_MEMORY_USAGE_DESCRIPTION);
+ builder.register(BrokerMetricNames.DISK_STORE_USAGE, this, metrics -> Double.valueOf(getPagingManager().getDiskTotalSpace() - getPagingManager().getDiskUsableSpace()), ActiveMQServerControl.DISK_STORE_USAGE_DESCRIPTION);
Review comment:
Typically the words used for disk reporting are used and free to avoid any confusion.
Youre using the word usage which to me suggest its the used space not the free
I would recommend we use clearer words like used and free
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r393213356
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##########
@@ -707,6 +707,24 @@ public long getAddressMemoryUsage() {
}
}
+ @Override
+ public long getDiskStoreUsed() {
Review comment:
Ultimately the calls here are using [`java.nio.file.FileStore#getUsableSpace`](https://docs.oracle.com/javase/8/docs/api/java/nio/file/FileStore.html#getUsableSpace--) and [`java.nio.file.FileStore#getTotalSpace`](https://docs.oracle.com/javase/8/docs/api/java/nio/file/FileStore.html#getTotalSpace--) so the math looks right.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-597212940
The failure is caused by a race condition from #3003. I believe that's now been resolved via #3009. Please rebase and squash your commits.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-605636964
@clebertsuconic Updated, please see and let me know if it looks fine now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r392378873
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##########
@@ -707,6 +707,24 @@ public long getAddressMemoryUsage() {
}
}
+ @Override
+ public long getDiskStoreUsed() {
Review comment:
If I understand correctly, disk usable represents the available space. Subtracting that from the total disk should give the disk used?
There is no documentation for disk usable method so I might be wrong.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris edited a comment on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris edited a comment on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595856041
The test failed again -- doesnt seem associated to this PR. Is there a way I can help fix this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595856041
The test failed again -- doesnt seem associated
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] asfgit closed pull request #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595791771
sorry.. I closed it by accident.. reopened it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r393217819
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/metrics/BrokerMetricNames.java
##########
@@ -22,5 +22,5 @@
public static final String CONNECTION_COUNT = "connection.count";
public static final String TOTAL_CONNECTION_COUNT = "total.connection.count";
public static final String ADDRESS_MEMORY_USAGE = "address.memory.usage";
-
+ public static final String DISK_STORE_USED = "disk.store.usage";
Review comment:
Your names & descriptions alternate between "used" and "usage". You should use the same verbiage everywhere to avoid confusion. For example, this line could be `DISK_STORE_USED = "disk.store.used"` or `DISK_STORE_USAGE = "disk.store.usage"`. I would go with "usage" since the address memory metric already follows this pattern.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r393215527
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
##########
@@ -726,6 +744,25 @@ public int getAddressMemoryUsagePercentage() {
return (int) result;
}
+ @Override
+ public int getDiskUsedStorePercentage() {
+ if (AuditLogger.isEnabled()) {
+ AuditLogger.getDiskStoreUsagePercentage(this.server);
+ }
+ long globalMaxSize = getGlobalMaxSize();
+ // no max size set implies 0% used
+ if (globalMaxSize <= 0) {
+ return 0;
+ }
+
+ long diskUsed = getDiskStoreUsed();
+ if (diskUsed <= 0) {
+ return 0;
+ }
+ double result = (100D * diskUsed) / globalMaxSize;
Review comment:
You should use `org.apache.activemq.artemis.core.server.files.FileStoreMonitor#calculateUsage` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull
request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r391132274
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2996,6 +2996,7 @@ private void registerMeters() {
builder.register(BrokerMetricNames.CONNECTION_COUNT, this, metrics -> Double.valueOf(getConnectionCount()), ActiveMQServerControl.CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.TOTAL_CONNECTION_COUNT, this, metrics -> Double.valueOf(getTotalConnectionCount()), ActiveMQServerControl.TOTAL_CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.ADDRESS_MEMORY_USAGE, this, metrics -> Double.valueOf(getPagingManager().getGlobalSize()), ActiveMQServerControl.ADDRESS_MEMORY_USAGE_DESCRIPTION);
+ builder.register(BrokerMetricNames.DISK_STORE_USAGE, this, metrics -> Double.valueOf(getPagingManager().getDiskTotalSpace() - getPagingManager().getDiskUsableSpace()), ActiveMQServerControl.DISK_STORE_USAGE_DESCRIPTION);
Review comment:
You are duplicating the attributes here?
Also DISK_STORE_USAGE.. this seems to give you the amount of free space available, instead of DISK Usage. Isn't that a mistake?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r391192987
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2996,6 +2996,7 @@ private void registerMeters() {
builder.register(BrokerMetricNames.CONNECTION_COUNT, this, metrics -> Double.valueOf(getConnectionCount()), ActiveMQServerControl.CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.TOTAL_CONNECTION_COUNT, this, metrics -> Double.valueOf(getTotalConnectionCount()), ActiveMQServerControl.TOTAL_CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.ADDRESS_MEMORY_USAGE, this, metrics -> Double.valueOf(getPagingManager().getGlobalSize()), ActiveMQServerControl.ADDRESS_MEMORY_USAGE_DESCRIPTION);
+ builder.register(BrokerMetricNames.DISK_STORE_USAGE, this, metrics -> Double.valueOf(getPagingManager().getDiskTotalSpace() - getPagingManager().getDiskUsableSpace()), ActiveMQServerControl.DISK_STORE_USAGE_DESCRIPTION);
Review comment:
Isnt disk usable space the free space?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] criew commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
criew commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r389546794
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java
##########
@@ -28,6 +28,7 @@
String CONNECTION_COUNT_DESCRIPTION = "Number of clients connected to this server";
String TOTAL_CONNECTION_COUNT_DESCRIPTION = "Number of clients which have connected to this server since it was started";
String ADDRESS_MEMORY_USAGE_DESCRIPTION = "Memory used by all the addresses on broker for in-memory messages";
+ String DISK_STORE_USAGE_DESCRIPTION = "Memory used by the disk store";
Review comment:
Space / bytes, not memory? Also JavaDoc comments below.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic closed pull request #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic closed pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596143333
checking build histories of master on travis, seems as soon as PR #3003 was merged, the build starting failing with the issue, as such does look like it is related to PR #3003
here is the build on master straight after 3003 was merged
https://travis-ci.org/apache/activemq-artemis/builds/658977926?utm_source=github_status&utm_medium=notification
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596143770
Actually seems since 2999 was merged https://travis-ci.org/apache/activemq-artemis/builds/658977370?utm_source=github_status&utm_medium=notification
there seems to been a number of merges very close together as such could be any that were merged before 2999 as unfortunately as merges so close together previous builds cancelled for next.
merges between last success build and first build failed, as such these are the smoking guns
2973, 2992, 2993, 2997, 2999
<img width="1066" alt="Screenshot 2020-03-07 at 22 35 11" src="https://user-images.githubusercontent.com/1387822/76153442-f09e0b00-60c3-11ea-8296-d59c62143191.png">
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] jbertram commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
jbertram commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-595820816
I did some work in `org.apache.activemq.cli.test.MessageSerializerTest` for PR #3003, but every test in that class passes for me locally (both with and without the commit from this PR). Perhaps it's a spurious failure?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-598307265
Rebased
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] criew commented on a change in pull request
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
criew commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r389547913
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2996,6 +2996,7 @@ private void registerMeters() {
builder.register(BrokerMetricNames.CONNECTION_COUNT, this, metrics -> Double.valueOf(getConnectionCount()), ActiveMQServerControl.CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.TOTAL_CONNECTION_COUNT, this, metrics -> Double.valueOf(getTotalConnectionCount()), ActiveMQServerControl.TOTAL_CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.ADDRESS_MEMORY_USAGE, this, metrics -> Double.valueOf(getPagingManager().getGlobalSize()), ActiveMQServerControl.ADDRESS_MEMORY_USAGE_DESCRIPTION);
+ builder.register(BrokerMetricNames.DISK_STORE_USAGE, this, metrics -> Double.valueOf(getPagingManager().getDiskTotalSpace() - getPagingManager().getDiskUsableSpace()), ActiveMQServerControl.DISK_STORE_USAGE_DESCRIPTION);
Review comment:
Add two more metrics - ADDRESS_MEMORY_USAGE_PERCENTAGE and DISK_STORE_USAGE_PERCENTAGE 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce commented on a change in pull
request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#discussion_r391205527
##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
##########
@@ -2996,6 +2996,7 @@ private void registerMeters() {
builder.register(BrokerMetricNames.CONNECTION_COUNT, this, metrics -> Double.valueOf(getConnectionCount()), ActiveMQServerControl.CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.TOTAL_CONNECTION_COUNT, this, metrics -> Double.valueOf(getTotalConnectionCount()), ActiveMQServerControl.TOTAL_CONNECTION_COUNT_DESCRIPTION);
builder.register(BrokerMetricNames.ADDRESS_MEMORY_USAGE, this, metrics -> Double.valueOf(getPagingManager().getGlobalSize()), ActiveMQServerControl.ADDRESS_MEMORY_USAGE_DESCRIPTION);
+ builder.register(BrokerMetricNames.DISK_STORE_USAGE, this, metrics -> Double.valueOf(getPagingManager().getDiskTotalSpace() - getPagingManager().getDiskUsableSpace()), ActiveMQServerControl.DISK_STORE_USAGE_DESCRIPTION);
Review comment:
Typically the words used for disk reporting are used and free to avoid any confusion.
Youre using the word usage which to me suggest its the used space not the freem
I would recommend we use clearer words like used and free
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] clebertsuconic commented on issue #3005:
ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-597266835
@atris you should rebase, and squash your commit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596142749
@atris i agree, i don't believe this is your PR. Master is failing with same issue.
@jbertram I'm getting test failures in CLI tests locally, when doing mvn clean install -Pfast-tests -Pextra-tests seems yours is the only change recently in the CLI area.
Failed tests:
MessageSerializerTest.testTextMessageImportExport:177->checkSentMessages:104->CliTestBase.consumeMessages:147 null
Tests in error:
MessageSerializerTest.testMapMessageImportExport:227 » InvalidDestination AMQ2...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue
#3005: ARTEMIS-2636: Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-596142749
@clebertsuconic i agree with @atris, i don't believe this PR is causing any failures. Master is failing with same issue.
@jbertram I'm getting test failures in CLI tests locally, when doing mvn clean install -Pfast-tests -Pextra-tests seems yours is the only change recently in the CLI area.
Failed tests:
MessageSerializerTest.testTextMessageImportExport:177->checkSentMessages:104->CliTestBase.consumeMessages:147 null
Tests in error:
MessageSerializerTest.testMapMessageImportExport:227 » InvalidDestination AMQ2...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-597205720
I dont understand the test failure -- the test passes locally for me
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [activemq-artemis] atris commented on issue #3005: ARTEMIS-2636:
Introduce Disk Usage Metrics
Posted by GitBox <gi...@apache.org>.
atris commented on issue #3005: ARTEMIS-2636: Introduce Disk Usage Metrics
URL: https://github.com/apache/activemq-artemis/pull/3005#issuecomment-597157886
> Thanks for adding the new features suggested in the ticket.
> I was not sure where to add my comments, so I created a review, hope that's ok.
> Would it be possible to add *_PERCENTAGE metrics as well?
Thanks, fixed your comments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services