ARTEMIS-2636: Introduce Disk Usage Metrics by atris · Pull Request #3005 · apache/artemis

atris · 2020-03-06T10:05:14Z

No description provided.

clebertsuconic · 2020-03-06T14:27:48Z

the PR test is failing.. can you check why it failed? running tests...

clebertsuconic · 2020-03-06T14:28:01Z

sorry.. I closed it by accident.. reopened it.

clebertsuconic · 2020-03-06T15:24:53Z

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.

clebertsuconic · 2020-03-06T15:25:13Z

can y ou run the test locally and let us know what you see?

jbertram · 2020-03-06T15:31:04Z

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?

atris · 2020-03-06T16:45:37Z

The test failed again -- doesnt seem associated to this PR. Is there a way I can help fix this?

michaelandrepearce · 2020-03-07T22:13:16Z

@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...

michaelandrepearce · 2020-03-07T22:21:29Z

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

michaelandrepearce · 2020-03-07T22:27:57Z

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

criew

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?

...ent/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java

criew · 2020-03-09T09:36:32Z

...is-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java

            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);


Add two more metrics - ADDRESS_MEMORY_USAGE_PERCENTAGE and DISK_STORE_USAGE_PERCENTAGE here

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

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?

Isnt disk usable space the free space?

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

atris · 2020-03-10T15:43:56Z

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.

atris · 2020-03-10T17:13:25Z

I dont understand the test failure -- the test passes locally for me

jbertram · 2020-03-10T17:27:14Z

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.

atris · 2020-03-10T17:45:12Z

Done, please see.

clebertsuconic · 2020-03-10T19:19:26Z

@atris you should rebase, and squash your commit

atris · 2020-03-11T04:10:19Z

@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?)

atris · 2020-03-12T17:12:09Z

Rebased

michaelandrepearce · 2020-03-12T17:50:53Z

Have you sorted better naming so its clearer? See inline comments by clebert and myself

...rc/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java

...-server/src/main/java/org/apache/activemq/artemis/core/server/metrics/BrokerMetricNames.java

atris · 2020-03-22T14:05:48Z

@jbertram @clebertsuconic @criew Fixed, please see and let me know your comments

clebertsuconic · 2020-03-23T14:13:44Z

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

ARTEMIS-2636: This commit introduces metrics to publish the amount of disk used currently

atris · 2020-03-29T13:38:03Z

@clebertsuconic Updated, please see and let me know if it looks fine now

clebertsuconic closed this Mar 6, 2020

clebertsuconic reopened this Mar 6, 2020

criew reviewed Mar 9, 2020

View reviewed changes

atris force-pushed the 2636 branch from bb9c57f to be3cdb1 Compare March 10, 2020 15:10

atris force-pushed the 2636 branch from b54f40b to 1abbcd2 Compare March 10, 2020 17:41

atris force-pushed the 2636 branch from 1abbcd2 to 0ad00a5 Compare March 12, 2020 17:11

atris force-pushed the 2636 branch from 0ad00a5 to 557a96e Compare March 13, 2020 17:36

clebertsuconic reviewed Mar 13, 2020

View reviewed changes

...rc/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java Outdated Show resolved Hide resolved

jbertram reviewed Mar 16, 2020

View reviewed changes

...rc/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java Outdated Show resolved Hide resolved

jbertram reviewed Mar 16, 2020

View reviewed changes

...-server/src/main/java/org/apache/activemq/artemis/core/server/metrics/BrokerMetricNames.java Outdated Show resolved Hide resolved

atris force-pushed the 2636 branch 2 times, most recently from 92aa80e to 13b2223 Compare March 22, 2020 14:05

ARTEMIS-2636: Introduce Disk Usage Metrics

bf1cc95

ARTEMIS-2636: This commit introduces metrics to publish the amount of disk used currently

atris force-pushed the 2636 branch from 13b2223 to bf1cc95 Compare March 29, 2020 13:34

asfgit closed this in 5518bcb Mar 31, 2020

Conversation

atris commented Mar 6, 2020

Uh oh!

clebertsuconic commented Mar 6, 2020

Uh oh!

clebertsuconic commented Mar 6, 2020

Uh oh!

clebertsuconic commented Mar 6, 2020

Uh oh!

clebertsuconic commented Mar 6, 2020

Uh oh!

jbertram commented Mar 6, 2020

Uh oh!

atris commented Mar 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelandrepearce commented Mar 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelandrepearce commented Mar 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelandrepearce commented Mar 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

criew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

criew Mar 9, 2020

Choose a reason for hiding this comment

Uh oh!

atris Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

clebertsuconic Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

atris Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

michaelandrepearce Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atris commented Mar 10, 2020

Uh oh!

atris commented Mar 10, 2020

Uh oh!

jbertram commented Mar 10, 2020

Uh oh!

atris commented Mar 10, 2020

Uh oh!

clebertsuconic commented Mar 10, 2020

Uh oh!

atris commented Mar 11, 2020

Uh oh!

atris commented Mar 12, 2020

Uh oh!

michaelandrepearce commented Mar 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

atris commented Mar 22, 2020

Uh oh!

clebertsuconic commented Mar 23, 2020

Uh oh!

atris commented Mar 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

atris commented Mar 6, 2020 •

edited

Loading

michaelandrepearce commented Mar 7, 2020 •

edited

Loading

michaelandrepearce commented Mar 7, 2020 •

edited

Loading

michaelandrepearce commented Mar 7, 2020 •

edited

Loading

michaelandrepearce Mar 11, 2020 •

edited

Loading