This is an automated email from the ASF dual-hosted git repository.
agingade pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 4e1c63c GEODE-3869: Fix early eviction. (#1152)
4e1c63c is described below
commit 4e1c63ca1eab154f24c9a79757e9600496494224
Author: agingade <aging...@pivotal.io>
AuthorDate: Tue Dec 12 14:11:25 2017 -0800
GEODE-3869: Fix early eviction. (#1152)
* GEODE-3869: Fix early eviction.
While code review found that the refactoring moved
"evictionNode.unsetRecentlyUsed()" to
"appendEntry()" which gets call by other places (adding entry into the
list); this causes
the recently used entry to be evicted early.
---
.../cache/eviction/AbstractEvictionList.java | 2 -
.../cache/eviction/LRUListWithAsyncSorting.java | 1 +
.../cache/eviction/LRUListWithSyncSorting.java | 1 +
.../cache/eviction/AbstractEvictionListTest.java | 1 -
.../eviction/LRUListWithAsyncSortingTest.java | 12 ++---
.../LRUListWithSyncSortingIntegrationTest.java | 3 +-
.../RegionEntryEvictionIntegrationTest.java | 63 ++++++++++++++++++++++
7 files changed, 73 insertions(+), 10 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/AbstractEvictionList.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/AbstractEvictionList.java
index b9ed858..dc9fc7b 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/AbstractEvictionList.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/AbstractEvictionList.java
@@ -116,8 +116,6 @@ abstract class AbstractEvictionList implements EvictionList
{
return;
}
- evictionNode.unsetRecentlyUsed();
-
if (logger.isTraceEnabled(LogMarker.LRU_CLOCK)) {
logger.trace(LogMarker.LRU_CLOCK, LocalizedMessage
.create(LocalizedStrings.NewLRUClockHand_ADDING_ANODE_TO_LRU_LIST,
evictionNode));
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSorting.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSorting.java
index 419b056..a14d363 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSorting.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSorting.java
@@ -127,6 +127,7 @@ public class LRUListWithAsyncSorting extends
AbstractEvictionList {
if (evictionNode.isRecentlyUsed() && evictionAttempts <
MAX_EVICTION_ATTEMPTS) {
evictionAttempts++;
+ evictionNode.unsetRecentlyUsed();
appendEntry(evictionNode);
continue;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSorting.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSorting.java
index 7f7e9ee..3577894 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSorting.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSorting.java
@@ -88,6 +88,7 @@ public class LRUListWithSyncSorting extends
AbstractEvictionList {
logger.trace(LogMarker.LRU_CLOCK, LocalizedMessage
.create(LocalizedStrings.NewLRUClockHand_SKIPPING_RECENTLY_USED_ENTRY, aNode));
}
+ aNode.unsetRecentlyUsed();
appendEntry(aNode);
continue; // keep looking
} else {
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/AbstractEvictionListTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/AbstractEvictionListTest.java
index d0e072a..f25cc40 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/AbstractEvictionListTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/AbstractEvictionListTest.java
@@ -161,7 +161,6 @@ public class AbstractEvictionListTest {
EvictionNode node = mock(EvictionNode.class);
evictionList.appendEntry(node);
- verify(node).unsetRecentlyUsed();
verify(node).setNext(evictionList.tail);
verify(node).setPrevious(evictionList.head);
assertThat(evictionList.tail.previous()).isSameAs(node);
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSortingTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSortingTest.java
index 04ab3e9..e5c1d63 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSortingTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithAsyncSortingTest.java
@@ -177,9 +177,9 @@ public class LRUListWithAsyncSortingTest {
when(recentlyUsedNode.next()).thenReturn(list.tail);
list.incrementRecentlyUsed();
- // unsetRecentlyUsed() is called once on appendEntry(...) and once during
scan
+ // unsetRecentlyUsed() is called once during scan
await().atMost(10, TimeUnit.SECONDS)
- .until(() -> verify(recentlyUsedNode, times(2)).unsetRecentlyUsed());
+ .until(() -> verify(recentlyUsedNode, times(1)).unsetRecentlyUsed());
realExecutor.shutdown();
}
@@ -196,9 +196,9 @@ public class LRUListWithAsyncSortingTest {
when(recentlyUsedNode.next()).thenReturn(recentlyUsedNode);
list.incrementRecentlyUsed();
- // unsetRecentlyUsed() is called once on appendEntry(...) and once during
scan
+ // unsetRecentlyUsed() is called once during scan
await().atMost(10, TimeUnit.SECONDS)
- .until(() -> verify(recentlyUsedNode, times(2)).unsetRecentlyUsed());
+ .until(() -> verify(recentlyUsedNode, times(1)).unsetRecentlyUsed());
realExecutor.shutdown();
}
@@ -226,9 +226,9 @@ public class LRUListWithAsyncSortingTest {
when(recentlyUsedNode.isRecentlyUsed()).thenReturn(true);
list.incrementRecentlyUsed();
- // unsetRecentlyUsed() is called once on appendEntry(...) and once during
scan
+ // unsetRecentlyUsed() is called once during scan
await().atMost(10, TimeUnit.SECONDS)
- .until(() -> verify(recentlyUsedNode, times(2)).unsetRecentlyUsed());
+ .until(() -> verify(recentlyUsedNode, times(1)).unsetRecentlyUsed());
assertThat(list.tail.previous()).isEqualTo(recentlyUsedNode);
realExecutor.shutdown();
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSortingIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSortingIntegrationTest.java
index 0bb28b5..384b3fc 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSortingIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/LRUListWithSyncSortingIntegrationTest.java
@@ -149,7 +149,8 @@ public class LRUListWithSyncSortingIntegrationTest {
}
@Test
- public void evictsRecentlyUsedNodeIfOverLimit() throws Exception {
+ public void
getEvictableEntryReturnsRecentlyUsedNodeAfterSearchMaxEntriesExceeded()
+ throws Exception {
IntStream.range(10, 16).forEach(i -> {
EvictionNode node = new LRUTestEntry(i);
nodes.add(node);
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/RegionEntryEvictionIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/RegionEntryEvictionIntegrationTest.java
new file mode 100644
index 0000000..305ab81
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/eviction/RegionEntryEvictionIntegrationTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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.internal.cache.eviction;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class RegionEntryEvictionIntegrationTest {
+ private Region<String, String> region;
+
+ @Rule
+ public TestName testName = new TestName();
+
+ @Before
+ public void setUp() throws Exception {
+ region = createRegion();
+ }
+
+ @Test
+ public void verifyMostRecentEntryIsNotEvicted() {
+ region.create("one", "one");
+ region.create("twoo", "twoo");
+ region.put("one", "one");
+ region.put("twoo", "twoo");
+
+ region.create("threee", "threee");
+ assertThat(region.size()).isEqualTo(2);
+ assertThat(region.containsKey("threee")).isTrue();
+ }
+
+ private Region<String, String> createRegion() throws Exception {
+ Cache cache = new CacheFactory().set("locators", "").set("mcast-port",
"0").create();
+ return cache.<String, String>createRegionFactory(RegionShortcut.REPLICATE)
+ .setEvictionAttributes(EvictionAttributes.createLRUEntryAttributes(2))
+ .create(testName.getMethodName());
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <commits@geode.apache.org>'].
