alex-plekhanov commented on code in PR #11300:
URL: https://github.com/apache/ignite/pull/11300#discussion_r1551082839
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
Review Comment:
Redundant NL
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java:
##########
@@ -100,6 +100,21 @@
*
*/
public class IgniteCacheOffheapManagerImpl implements
IgniteCacheOffheapManager {
+ /** */
+ private static class RemovedFromPendingEntries extends KeyCacheObjectImpl {
Review Comment:
1. All inner classes must be at the end of the file.
2. Let's rename to something like `ExpiredKeyCacheObject` and add comment
that this class is a maker.
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
+ return new PageHandler<Object, BPlusTree.Result>() {
+ @Override public BPlusTree.Result run(int cacheId, long
pageId, long page, long pageAddr, PageIO io,
+ Boolean walPlc, Object arg, int lvl,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
+
calls.merge(StringUtils.substringAfterLast(hnd.getClass().getName(), "$"), 1,
Integer::sum);
+
+ return ((PageHandler<Object,
BPlusTree.Result>)hnd).run(cacheId, pageId, page, pageAddr, io,
+ walPlc, arg, lvl, statHolder);
+ }
+
+ @Override public boolean releaseAfterWrite(int cacheId,
long pageId, long page, long pageAddr, Object arg,
Review Comment:
Codestyle violation, see
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-MethodParameters
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
+ return new PageHandler<Object, BPlusTree.Result>() {
+ @Override public BPlusTree.Result run(int cacheId, long
pageId, long page, long pageAddr, PageIO io,
+ Boolean walPlc, Object arg, int lvl,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
+
calls.merge(StringUtils.substringAfterLast(hnd.getClass().getName(), "$"), 1,
Integer::sum);
+
+ return ((PageHandler<Object,
BPlusTree.Result>)hnd).run(cacheId, pageId, page, pageAddr, io,
+ walPlc, arg, lvl, statHolder);
+ }
+
+ @Override public boolean releaseAfterWrite(int cacheId,
long pageId, long page, long pageAddr, Object arg,
+ int intArg) {
+ return ((PageHandler<Object, BPlusTree.Result>)hnd)
+ .releaseAfterWrite(cacheId, pageId, page,
pageAddr, arg, intArg);
+ }
+ };
+ }
+
+ return hnd;
+ };
+
+ try (IgniteKernal g = (IgniteKernal)startGrid(0)) {
+ final String key = "key";
+
+ final int records = 1500;
+
+ assertEquals(0,
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize());
+
+ IgniteCache<Object, Object> cache =
g.cache(DEFAULT_CACHE_NAME).withExpiryPolicy(
+ new CreatedExpiryPolicy(new Duration(MILLISECONDS, 1000)));
+
+ IntStream.range(0, records).forEach(x -> cache.put(key + x, x));
+
+ assertEquals(records,
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize());
+
+ U.sleep(2000);
+
+ assertEquals(0,
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize());
Review Comment:
Let's replace to something like:
```
assertTrue(GridTestUtils.waitForCondition(
() -> {
try {
return
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize() ==
0;
}
catch (Exception ignore) {
return false;
}
}, 5_000L)
);
```
2 secs it's to risky and test can become flaky.
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
+ return new PageHandler<Object, BPlusTree.Result>() {
+ @Override public BPlusTree.Result run(int cacheId, long
pageId, long page, long pageAddr, PageIO io,
+ Boolean walPlc, Object arg, int lvl,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
+
calls.merge(StringUtils.substringAfterLast(hnd.getClass().getName(), "$"), 1,
Integer::sum);
Review Comment:
`StringUtils.substringAfterLast(hnd.getClass().getName(), "$")` ->
`hnd.getClass().getSimpleName()`
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
+ return new PageHandler<Object, BPlusTree.Result>() {
+ @Override public BPlusTree.Result run(int cacheId, long
pageId, long page, long pageAddr, PageIO io,
Review Comment:
Codestyle violation, see
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-MethodParameters
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
+ return new PageHandler<Object, BPlusTree.Result>() {
+ @Override public BPlusTree.Result run(int cacheId, long
pageId, long page, long pageAddr, PageIO io,
+ Boolean walPlc, Object arg, int lvl,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
+
calls.merge(StringUtils.substringAfterLast(hnd.getClass().getName(), "$"), 1,
Integer::sum);
+
+ return ((PageHandler<Object,
BPlusTree.Result>)hnd).run(cacheId, pageId, page, pageAddr, io,
+ walPlc, arg, lvl, statHolder);
+ }
+
+ @Override public boolean releaseAfterWrite(int cacheId,
long pageId, long page, long pageAddr, Object arg,
+ int intArg) {
+ return ((PageHandler<Object, BPlusTree.Result>)hnd)
+ .releaseAfterWrite(cacheId, pageId, page,
pageAddr, arg, intArg);
+ }
+ };
+ }
+
+ return hnd;
+ };
+
+ try (IgniteKernal g = (IgniteKernal)startGrid(0)) {
Review Comment:
IgniteEx g = startGrid(0)
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/GridCacheTtlManagerSelfTest.java:
##########
@@ -104,4 +117,86 @@ private void checkTtl(CacheMode mode) throws Exception {
stopAllGrids();
}
}
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testPartitionedRemove() throws Exception {
+ checkRemove(PARTITIONED);
+ }
+
+ /**
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testReplicatedRemove() throws Exception {
+ checkRemove(REPLICATED);
+ }
+
+ /**
+ * @param mode Cache mode.
+ * @throws Exception If failed.
+ */
+ private void checkRemove(CacheMode mode) throws Exception {
+ cacheMode = mode;
+
+ Map<String, Integer> calls = new ConcurrentHashMap<>();
+
+ BPlusTree.testHndWrapper = (tree, hnd) -> {
+ if (tree instanceof PendingEntriesTree) {
+
+ return new PageHandler<Object, BPlusTree.Result>() {
+ @Override public BPlusTree.Result run(int cacheId, long
pageId, long page, long pageAddr, PageIO io,
+ Boolean walPlc, Object arg, int lvl,
IoStatisticsHolder statHolder) throws IgniteCheckedException {
+
calls.merge(StringUtils.substringAfterLast(hnd.getClass().getName(), "$"), 1,
Integer::sum);
+
+ return ((PageHandler<Object,
BPlusTree.Result>)hnd).run(cacheId, pageId, page, pageAddr, io,
+ walPlc, arg, lvl, statHolder);
+ }
+
+ @Override public boolean releaseAfterWrite(int cacheId,
long pageId, long page, long pageAddr, Object arg,
+ int intArg) {
+ return ((PageHandler<Object, BPlusTree.Result>)hnd)
+ .releaseAfterWrite(cacheId, pageId, page,
pageAddr, arg, intArg);
+ }
+ };
+ }
+
+ return hnd;
+ };
+
+ try (IgniteKernal g = (IgniteKernal)startGrid(0)) {
+ final String key = "key";
+
+ final int records = 1500;
+
+ assertEquals(0,
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize());
+
+ IgniteCache<Object, Object> cache =
g.cache(DEFAULT_CACHE_NAME).withExpiryPolicy(
+ new CreatedExpiryPolicy(new Duration(MILLISECONDS, 1000)));
+
+ IntStream.range(0, records).forEach(x -> cache.put(key + x, x));
+
+ assertEquals(records,
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize());
+
+ U.sleep(2000);
+
+ assertEquals(0,
g.context().cache().cache(DEFAULT_CACHE_NAME).context().ttl().pendingSize());
+
+ log.info("Invocation counts\n" + calls.keySet().stream()
+ .map(k -> k + ": " + calls.get(k))
+ .collect(Collectors.joining("\n")));
+
+ assertTrue(calls.get("Insert") >= records);
+ assertTrue(calls.get("RemoveRangeFromLeaf") > 2);
+ assertTrue(calls.get("RemoveRangeFromLeaf") < 20);
+ assertTrue(calls.get("Search") < 3000);
Review Comment:
`Search` can be ran for `RemoveRange` and for `Remove` as well. Perhaps we
should store class of `arg` instead of class of `hnd`. In this case we will get
`Put`, `Remove` and `RemoveRange` map keys.
Also, i'm not sure we should check exact number of calls. For `Remove` and
`RemoveRange` I think we can check that calls map contains `RemoveRange` and
doesn't contain `Remove` keys.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
