[
https://issues.apache.org/jira/browse/GEODE-1897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16257096#comment-16257096
]
ASF GitHub Bot commented on GEODE-1897:
---------------------------------------
jdeppe-pivotal closed pull request #1068: GEODE-1897: Ensure that
eviction-object-sizer also implements Declarable
URL: https://github.com/apache/geode/pull/1068
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/cli/commands/CreateRegionCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
index a4b2f72883..5eeed3f637 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
@@ -678,15 +678,9 @@ public Result preExecution(GfshParseResult parseResult) {
.createUserErrorResult(CliStrings.CREATE_REGION__MSG__MISSING_EVICTION_ACTION);
}
- if (evictionSizer != null) {
- if (maxEntry != null) {
- return ResultBuilder.createUserErrorResult(
-
CliStrings.CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_AND_ENTRY_COUNT);
- }
- if (maxMemory == null) {
- return ResultBuilder.createUserErrorResult(
-
CliStrings.CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_WITHOUT_MAX_MEMORY);
- }
+ if (evictionSizer != null && maxEntry != null) {
+ return ResultBuilder.createUserErrorResult(
+
CliStrings.CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_AND_ENTRY_COUNT);
}
if (evictionAction != null
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
index 55344d4e74..1c7baef4d1 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
@@ -24,6 +24,7 @@
import org.apache.geode.cache.CacheLoader;
import org.apache.geode.cache.CacheWriter;
import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.Declarable;
import org.apache.geode.cache.EvictionAttributes;
import org.apache.geode.cache.PartitionAttributes;
import org.apache.geode.cache.PartitionAttributesFactory;
@@ -36,6 +37,7 @@
import org.apache.geode.cache.execute.Function;
import org.apache.geode.cache.execute.FunctionContext;
import org.apache.geode.cache.execute.ResultSender;
+import org.apache.geode.cache.util.ObjectSizer;
import org.apache.geode.compression.Compressor;
import org.apache.geode.internal.ClassPathLoader;
import org.apache.geode.internal.InternalEntity;
@@ -202,6 +204,11 @@ private CliFunctionResult handleException(final String
memberNameOrId, final Str
EvictionAttributes evictionAttributes =
regionCreateArgs.getEvictionAttributes();
if (evictionAttributes != null) {
+ ObjectSizer sizer = evictionAttributes.getObjectSizer();
+ if (sizer != null && !(sizer instanceof Declarable)) {
+ throw new IllegalArgumentException(
+
CliStrings.CREATE_REGION__MSG__OBJECT_SIZER_MUST_BE_OBJECTSIZER_AND_DECLARABLE);
+ }
factory.setEvictionAttributes(evictionAttributes);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index 0e27fba208..cf99d0f202 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -869,7 +869,7 @@
"Activates LRU eviction based on the region's memory usage specified by
this value.";
public static final String CREATE_REGION__EVICTION_ENTRY_COUNT =
"eviction-entry-count";
public static final String CREATE_REGION__EVICTION_ENTRY_COUNT__HELP =
- "Activates LRU eviction based on the region's entry count specified by
this value.";
+ "Activates LRU eviction based on the region's entry count specified by
this value (in megabytes).";
public static final String CREATE_REGION__EVICTION_OBJECT_SIZER =
"eviction-object-sizer";
public static final String CREATE_REGION__EVICTION_OBJECT_SIZER__HELP =
"A custom class which implements ObjectSizer in order to perform max
memory eviction.";
@@ -1068,6 +1068,9 @@
public static final String
CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_WITHOUT_MAX_MEMORY =
"eviction-object-sizer cannot be specified without eviction-max-memory";
+ public static final String
CREATE_REGION__MSG__OBJECT_SIZER_MUST_BE_OBJECTSIZER_AND_DECLARABLE =
+ "eviction-object-sizer must implement both ObjectSizer and Declarable
interfaces";
+
/* debug command */
public static final String DEBUG = "debug";
public static final String DEBUG__HELP = "Enable/Disable debugging output in
GFSH.";
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
index b9f6513dcf..01b5a7add8 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
@@ -19,7 +19,8 @@
import java.io.File;
-import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -42,14 +43,13 @@
@Category(DistributedTest.class)
public class CreateRegionCommandDUnitTest {
- MemberVM locator;
- MemberVM server;
+ private static MemberVM locator, server;
- @Rule
- public LocatorServerStartupRule lsRule = new LocatorServerStartupRule();
+ @ClassRule
+ public static LocatorServerStartupRule lsRule = new
LocatorServerStartupRule();
- @Rule
- public GfshCommandRule gfsh = new GfshCommandRule();
+ @ClassRule
+ public static GfshCommandRule gfsh = new GfshCommandRule();
@Rule
public TestName testName = new SerializableTestName();
@@ -57,8 +57,8 @@
@Rule
public TemporaryFolder tmpDir = new TemporaryFolder();
- @Before
- public void before() throws Exception {
+ @BeforeClass
+ public static void before() throws Exception {
locator = lsRule.startLocatorVM(0);
server = lsRule.startServerVM(1, locator.getPort());
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
index 28dffab734..de80f75fbd 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
@@ -509,6 +509,24 @@ public void testEvictionAttributesForLRUHeap() throws
Exception {
gfsh.executeAndAssertThat("destroy region --name=/FOO").statusIsSuccess();
}
+ @Test
+ public void testEvictionAttributesForLRUHeapWithObjectSizer() throws
Exception {
+ gfsh.executeAndAssertThat(
+ "create region --name=FOO --type=REPLICATE
--eviction-action=local-destroy --eviction-object-sizer="
+ + TestObjectSizer.class.getName())
+ .statusIsSuccess();
+
+ Region foo = server.getCache().getRegion("/FOO");
+ assertThat(foo.getAttributes().getEvictionAttributes().getAction())
+ .isEqualTo(EvictionAction.LOCAL_DESTROY);
+ assertThat(foo.getAttributes().getEvictionAttributes().getAlgorithm())
+ .isEqualTo(EvictionAlgorithm.LRU_HEAP);
+
assertThat(foo.getAttributes().getEvictionAttributes().getObjectSizer().getClass().getName())
+ .isEqualTo(TestObjectSizer.class.getName());
+
+ gfsh.executeAndAssertThat("destroy region --name=/FOO").statusIsSuccess();
+ }
+
@Test
public void testEvictionAttributesForLRUEntry() throws Exception {
gfsh.executeAndAssertThat(
@@ -542,7 +560,7 @@ public void testEvictionAttributesForLRUMemory() throws
Exception {
}
@Test
- public void testEvictionAttributesForSizer() throws Exception {
+ public void testEvictionAttributesForObjectSizer() throws Exception {
gfsh.executeAndAssertThat(
"create region --name=FOO --type=REPLICATE --eviction-max-memory=1001
--eviction-action=overflow-to-disk --eviction-object-sizer="
+ TestObjectSizer.class.getName())
@@ -558,4 +576,13 @@ public void testEvictionAttributesForSizer() throws
Exception {
gfsh.executeAndAssertThat("destroy region --name=/FOO").statusIsSuccess();
}
+
+ @Test
+ public void testEvictionAttributesForNonDeclarableObjectSizer() throws
Exception {
+ gfsh.executeAndAssertThat(
+ "create region --name=FOO --type=REPLICATE --eviction-max-memory=1001
--eviction-action=overflow-to-disk --eviction-object-sizer="
+ + TestObjectSizerNotDeclarable.class.getName())
+ .statusIsError().containsOutput(
+ "eviction-object-sizer must implement both ObjectSizer and
Declarable interfaces");
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
index d320386b97..4b360d41cd 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
@@ -124,15 +124,6 @@ public void invalidEvictionSizerAndCount() throws
Exception {
.contains("eviction-object-sizer cannot be specified with
eviction-entry-count");
}
- @Test
- public void invalidEvictionSizerWithoutMemory() throws Exception {
- CommandResult result = parser.executeCommandWithInstance(command,
- "create region --name=region --type=REPLICATE
--eviction-object-sizer=abc --eviction-action=local-destroy");
- assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
- assertThat(result.getContent().toString())
- .contains("eviction-object-sizer cannot be specified without
eviction-max-memory");
- }
-
@Test
public void templateRegionAttributesNotAvailable() throws Exception {
doReturn(null).when(command).getRegionAttributes(eq(cache), any());
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/TestObjectSizerNotDeclarable.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/TestObjectSizerNotDeclarable.java
new file mode 100644
index 0000000000..b16be55f51
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/TestObjectSizerNotDeclarable.java
@@ -0,0 +1,25 @@
+/*
+ * 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.geode.management.internal.cli.commands;
+
+import org.apache.geode.cache.util.ObjectSizer;
+
+public class TestObjectSizerNotDeclarable implements ObjectSizer {
+ @Override
+ public int sizeof(Object o) {
+ return 77;
+ }
+}
----------------------------------------------------------------
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:
[email protected]
> Users should be able to configure eviction through gfsh
> -------------------------------------------------------
>
> Key: GEODE-1897
> URL: https://issues.apache.org/jira/browse/GEODE-1897
> Project: Geode
> Issue Type: Sub-task
> Components: docs, gfsh
> Reporter: Swapnil Bawaskar
> Assignee: Jens Deppe
>
> While creating a region in gfsh, users should be able to configure eviction
> for that region.
> All three modes of eviction should be supported:
> 1. Eviction driven by the resource manager:
> {noformat}
> gfsh>create region --name=myRegion --type=REPLICATE --eviction-enabled
> {noformat}
> 2. eviction driven by entry count in the region:
> {noformat}
> gfsh>create region --name=myRegion --type=REPLICATE
> --eviction-entry-count=1000
> {noformat}
> 3. eviction driven by bytes used:
> {noformat}
> gfsh>create region --name=myRegion --type=REPLICATE --eviction-max-memory=100m
> {noformat}
> And also specify the eviction action as
> {noformat}
> --eviction-action=overflow-to-disk or
> --eviction-action=destroy
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
