[
https://issues.apache.org/jira/browse/GEODE-2676?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16259410#comment-16259410
]
ASF GitHub Bot commented on GEODE-2676:
---------------------------------------
jinmeiliao closed pull request #1063: GEODE-2676: fix NPE with
ShowMetricsCommand.
URL: https://github.com/apache/geode/pull/1063
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/RegionMBeanBridge.java
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/RegionMBeanBridge.java
index 480c5c0dcf..93da8b42e0 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/RegionMBeanBridge.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/RegionMBeanBridge.java
@@ -145,6 +145,13 @@ protected RegionMBeanBridge(Region<K, V> region) {
this.regAttrs = region.getAttributes();
this.isStatisticsEnabled = regAttrs.getStatisticsEnabled();
+ if (isStatisticsEnabled) {
+ try {
+ region.getStatistics();
+ } catch (UnsupportedOperationException e) {
+ this.isStatisticsEnabled = false;
+ }
+ }
this.regionAttributesData =
RegionMBeanCompositeDataFactory.getRegionAttributesData(regAttrs);
this.membershipAttributesData =
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommandIntegrationTest.java
index b18af6ccbf..7bbad90a62 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommandIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMetricsCommandIntegrationTest.java
@@ -45,10 +45,10 @@
@ClassRule
public static ServerStarterRule server =
new ServerStarterRule().withRegion(RegionShortcut.REPLICATE, REGION_NAME)
-
.withName(MEMBER_NAME).withJMXManager().withEmbeddedLocator().withAutoStart();
+ .withName(MEMBER_NAME).withJMXManager().withAutoStart();
@Rule
- public GfshCommandRule gfsh = new GfshCommandRule();
+ public GfshCommandRule gfsh = new GfshCommandRule(server::getJmxPort,
PortType.jmxManager);
@Test
public void everyCategoryHasAUseCase() throws Exception {
@@ -68,6 +68,7 @@ public void everyCategoryHasAUseCase() throws Exception {
@Test
public void commandFailsWhenNotConnected() throws Exception {
+ gfsh.disconnect();
gfsh.executeAndAssertThat("show metrics")
.containsOutput("was found but is not currently available");
}
@@ -80,9 +81,7 @@ public void getRegionMetricsShowsExactlyDefaultCategories()
throws Exception {
ShowMetricsInterceptor.getValidCategoriesAsStrings(true, true, false);
// Blank lines are permitted for grouping.
expectedCategories.add("");
- logger.info("Expecting categories: " + String.join(", ",
expectedCategories));
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).tableHasColumnOnlyWithValues("Category",
expectedCategories.toArray(new String[0]));
}
@@ -97,7 +96,6 @@ public void
getSystemRegionMetricsShowsExactlyDefaultCategories() throws Excepti
expectedCategories.add("");
logger.info("Expecting categories: " + String.join(", ",
expectedCategories));
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).tableHasColumnOnlyWithValues("Category",
expectedCategories.toArray(new String[0]));
}
@@ -112,7 +110,6 @@ public void getMemberMetricsShowsExactlyDefaultCategories()
throws Exception {
expectedCategories.add("");
logger.info("Expecting categories: " + String.join(", ",
expectedCategories));
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).tableHasColumnOnlyWithValues("Category",
expectedCategories.toArray(new String[0]));
}
@@ -127,7 +124,6 @@ public void
getMemberWithPortMetricsShowsExactlyDefaultCategories() throws Excep
expectedCategories.add("");
logger.info("Expecting categories: " + String.join(", ",
expectedCategories));
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).tableHasColumnOnlyWithValues("Category",
expectedCategories.toArray(new String[0]));
}
@@ -142,7 +138,6 @@ public void getSystemMetricsShowsExactlyDefaultCategories()
throws Exception {
expectedCategories.add("");
logger.info("Expecting categories: " + String.join(", ",
expectedCategories));
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).tableHasColumnOnlyWithValues("Category",
expectedCategories.toArray(new String[0]));
}
@@ -152,7 +147,6 @@ public void invalidCategoryGetsReported() throws Exception {
String cmd =
"show metrics
--categories=\"cluster,cache,some_invalid_category,another_invalid_category\"";
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).containsOutput("Invalid Categories")
.containsOutput("some_invalid_category").containsOutput("another_invalid_category")
.doesNotContainOutput("cache").doesNotContainOutput("cluster");
@@ -164,8 +158,16 @@ public void categoryOptionAbridgesOutput() throws
Exception {
List<String> expectedCategories = Arrays.asList("cluster", "cache", "");
logger.info("Expecting categories: " + String.join(", ",
expectedCategories));
- gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), PortType.locator);
gfsh.executeAndAssertThat(cmd).tableHasColumnOnlyWithValues("Category",
expectedCategories.toArray(new String[0]));
}
+
+ @Test
+ public void getRegionMetricsForPartitionedRegionWithStatistics() throws
Exception {
+ String cmd = "create region --name=region2 --type=PARTITION
--enable-statistics";
+ gfsh.executeAndAssertThat(cmd).statusIsSuccess();
+ String cmd2 = "show metrics --member=" + MEMBER_NAME + " --region=region2";
+
gfsh.executeAndAssertThat(cmd2).statusIsSuccess().tableHasRowWithValues("Category",
"Metric",
+ "Value", "", "missCount", "-1");
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
b/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
index 97c72dcdf3..3ca72794be 100644
---
a/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
+++
b/geode-core/src/test/java/org/apache/geode/test/junit/assertions/CommandResultAssert.java
@@ -17,12 +17,15 @@
import static org.assertj.core.api.Assertions.assertThat;
import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
import org.assertj.core.api.AbstractAssert;
import org.assertj.core.api.Assertions;
import org.json.JSONArray;
import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.json.GfJsonException;
import org.apache.geode.management.internal.cli.json.GfJsonObject;
import org.apache.geode.management.internal.cli.result.CommandResult;
@@ -181,6 +184,48 @@ public CommandResultAssert
tableHasColumnWithExactValuesInAnyOrder(String header
return this;
}
+
+
+ public CommandResultAssert tableHasRowWithValues(String... headersThenValues)
+ throws GfJsonException {
+ assertThat(headersThenValues.length % 2)
+ .describedAs("You need to pass even number of
parameters.").isEqualTo(0);
+
+ int numberOfColumn = headersThenValues.length / 2;
+
+ String[] headers = Arrays.copyOfRange(headersThenValues, 0,
numberOfColumn);
+ String[] expectedValues =
+ Arrays.copyOfRange(headersThenValues, numberOfColumn,
headersThenValues.length);
+
+ GfJsonObject content = actual.getCommandResult().getContent();
+ Map<String, Object[]> allValues = new HashMap<>();
+ int numberOfRows = -1;
+ for (String header : headers) {
+ Object[] values = toArray((JSONArray) getColumnContent(header, content));
+ if (numberOfRows > 0) {
+ assertThat(values.length).isEqualTo(numberOfRows);
+ }
+ numberOfRows = values.length;
+ allValues.put(header, values);
+ }
+
+ for (int rowIndex = 0; rowIndex < numberOfRows; rowIndex++) {
+ Object[] rowValues = new Object[headers.length];
+ for (int columnIndex = 0; columnIndex < headers.length; columnIndex++) {
+ rowValues[columnIndex] = allValues.get(headers[columnIndex])[rowIndex];
+ }
+
+ // check if entire row is equal, but if not, continue to next row
+ if (Arrays.deepEquals(expectedValues, rowValues)) {
+ return this;
+ }
+ }
+
+ // did not find any matching rows, then this would pass only if we do not
pass in any values
+ assertThat(headersThenValues.length).isEqualTo(0);
+ return this;
+ }
+
/**
* Verifies that each of the actual values in the column with the given
header contains at least
* one of the expectedValues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> RegionMBean statistics wrong on partitioned regions
> ---------------------------------------------------
>
> Key: GEODE-2676
> URL: https://issues.apache.org/jira/browse/GEODE-2676
> Project: Geode
> Issue Type: Bug
> Components: management
> Reporter: Fred Krone
> Priority: Minor
> Labels: jmx
>
> RegionMBean attributes hitCount, hitRatio, missCount, lastAccessedTime, and
> lastModifiedTime will always be 0 for an mbean that represents an partitioned
> region.
> The gettors for these methods may call getStatistics() which on a PR always
> throws UnsupportedOperationException. So this exception might even get
> exposed to customers.
> The initialization of RegionMBeanBridge calls getStatisticsEnabled() which
> returns true on a PartitionedRegion. This does have meaning on a PR but it
> does not mean that getStatistics() is a supported operation. On a PR setting
> statistics-enabled causes each region-entry to also keep track of its last
> access time.
> It is true that if getStatisticsEnabled() is false then you should not call
> getStatistics. But the opposite is not true. Since we currently have regions
> that do not support getStatistics(), the code in RegionMBeanBridge should
> catch UnsupportedOperationException and handle it. I would suggest that the
> constructor be changed that initializes the "isStatisticsEnabled" field.
> Instead of only calling getStatisticsEnabled() it should also call
> getStatistics(). Something like this:
> {noformat}
> {
> boolean useGetStatistics = regAttrs.getStatisticsEnabled();
> if (useGetStatistics) {
> try {
> region.getStatistics();
> } catch (UnsupportedOperationException ex) {
> useGetStatistics = false;
> }
> }
> this.isStatisticsEnabled = useGetStatistics;
> }
> {noformat}
> That way in a future release if PRs are changed to support getStatistics this
> code will start calling it without having a direct dependency on the
> implementation of PartitionedRegion.
> https://issues.apache.org/jira/browse/GEODE-2685 is a request to support
> getStatistics on PRs.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
