alex-plekhanov commented on code in PR #11357:
URL: https://github.com/apache/ignite/pull/11357#discussion_r1616700228


##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheGroupView.java:
##########
@@ -150,4 +152,21 @@ public int rebalanceOrder() {
     public int backups() {
         return ccfg.getBackups();
     }
+
+    /** @return {@code Yes} if group has expired entries, {@code No} 
otherwise. If {@code eagerTtl = true} returns 'Unknown'*/
+    public String hasEntriesPendingExpire() {

Review Comment:
   We don't need such a column in chage groups view if we have it in cache view.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java:
##########
@@ -1180,6 +1180,27 @@ private int expireInternal(
         return pendingEntries != null ? pendingEntries.size() : 0;
     }
 
+    /** {@inheritDoc} */
+    @Override public boolean hasEntriesPendingExpire() throws 
IgniteCheckedException {
+        return pendingEntries != null && !pendingEntries.isEmpty();
+    }
+
+    /** {@inheritDoc} */
+    @Override public boolean hasEntriesPendingExpire(int cacheId) throws 
IgniteCheckedException {
+        if (pendingEntries != null) {
+            PendingRow row = new PendingRow(cacheId);
+
+            GridCursor<PendingRow> cursor = pendingEntries.find(row, row, 
PendingEntriesTree.WITHOUT_KEY);
+
+            while (cursor.next()) {
+                if (pendingEntries.findOne(cursor.get()) != null)

Review Comment:
   Why do we need exctra call to `findOne`? Why just `return cursor.next();` is 
not enough?



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */

Review Comment:
   Point at the end of sentence.



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewCacheExpiryPolicyTest.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.ignite.internal.metric;
+
+import java.util.Arrays;
+import java.util.Collection;
+import javax.cache.expiry.Duration;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import 
org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicyFactory;
+import org.apache.ignite.spi.systemview.view.CacheView;
+import org.apache.ignite.spi.systemview.view.SystemView;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import static 
org.apache.ignite.internal.processors.cache.ClusterCachesInfo.CACHES_VIEW;
+
+/** Tests for {@link CacheView} expiry policy factory representation */
+@RunWith(Parameterized.class)
+public class SystemViewCacheExpiryPolicyTest extends GridCommonAbstractTest {
+    /**Create ttl {@link Duration}*/
+    @Parameterized.Parameter
+    public long create;
+
+    /** Access ttl {@link Duration}*/
+    @Parameterized.Parameter(1)
+    public long update;
+
+    /** Update ttl {@link Duration}*/
+    @Parameterized.Parameter(2)
+    public long access;
+
+    /** Anticipated {@link String} expiry policy factory representation*/
+    @Parameterized.Parameter(3)
+    public String actual;
+
+    /**
+     * @return Test parameters.
+     */
+    @Parameterized.Parameters(name = "create={0}, update={1}, access={2}, 
actual={3}")
+    public static Collection parameters() {
+        return Arrays.asList(new Object[][] {
+            {2, 4, 8, 
"[create=2MILLISECONDS][update=4MILLISECONDS][access=8MILLISECONDS]"},
+            {1, -2, -1, "[create=1MILLISECONDS]"},
+            {-1, 0, -1, "Eternal"},
+            {0, 1, -1, "[update=1MILLISECONDS]"}
+        });
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTestsStarted() throws Exception {
+        super.beforeTestsStarted();
+
+        cleanPersistenceDir();

Review Comment:
   Test doesn't use persistence. Methods `beforeTestsStarted`, 
`afterTestsStopped` are redundant.



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";

Review Comment:
   Use `StringBuilder` to reduce GC pressure



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/
+    @Test
+    public void testCacheViewExpiryPolicyWithPersist() throws Exception {
+        testCacheViewExpiryPolicy(true);
+    }
+
+    /** Tests work of {@link SystemView} for cache groups expiry policy info */
+    private void testCacheViewExpiryPolicy(boolean withPersistance) throws 
Exception {
+        try (IgniteEx g = !withPersistance ? startGrid() : 
startGrid(getConfiguration().setDataStorageConfiguration(
+            new DataStorageConfiguration().setDefaultDataRegionConfiguration(
+                new DataRegionConfiguration().setPersistenceEnabled(true)
+            )))) {
+
+            if (withPersistance)
+                g.cluster().state(ClusterState.ACTIVE);
+
+            String eternalCacheName = "eternalCache";
+            String createdCacheName = "createdCache";
+            String eagerTtlCacheName = "eagerTtlCache";
+            String withoutGrpCacheName = "withoutGrpCache";
+
+            CacheConfiguration<Long, Long> eternalCache = new 
CacheConfiguration<Long, Long>(eternalCacheName)
+                .setGroupName("group1")
+                .setExpiryPolicyFactory(EternalExpiryPolicy.factoryOf());
+
+            CacheConfiguration<Long, Long> createdCache = new 
CacheConfiguration<Long, Long>(createdCacheName)
+                .setGroupName("group2")
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            CacheConfiguration<Long, Long> eagerTtlCache = new 
CacheConfiguration<Long, Long>(eagerTtlCacheName)
+                .setGroupName("group2")
+                .setEagerTtl(false)
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            CacheConfiguration<Long, Long> withoutGrpCache = new 
CacheConfiguration<Long, Long>(withoutGrpCacheName)

Review Comment:
   Let's add cache without explicit expiry policy, but with expiring entry.



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";
+
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("create", 
expiryPlc.getExpiryForCreation());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("update", 
expiryPlc.getExpiryForUpdate());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("access", 
expiryPlc.getExpiryForAccess());
+
+        return expiryPlcFactoryStr.equals("") ? "Eternal" : 
expiryPlcFactoryStr;
+    }
+
+    /** @return {@code Yes} if group has expired entries, {@code No} 
otherwise. If {@code eagerTtl = true} returns 'Unknown'*/
+    public String hasEntriesPendingExpire() {
+        CacheGroupContext grpCtx = ctx.cache().cacheGroup(cache.groupId());
+
+        if (!cache.cacheConfiguration().isEagerTtl() || grpCtx == null || 
grpCtx.name() == null)
+            return "Unknown";
+
+        try {
+            return grpCtx.offheap().hasEntriesPendingExpire(cache.cacheId()) ? 
"Yes" : "No";
+        }
+        catch (IgniteCheckedException e) {
+            return e.getMessage();
+        }
+    }
+
+    /**
+     * Returns {@link Duration} String representation with description
+     * @param msg descriptive message for {@link Duration} instance
+     * @param duration @link Duration}
+     * @return {@link Duration} representation with descriptive message

Review Comment:
   Point at the end of each sentence.
   `@link Duration}` - wrong formatting



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";
+
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("create", 
expiryPlc.getExpiryForCreation());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("update", 
expiryPlc.getExpiryForUpdate());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("access", 
expiryPlc.getExpiryForAccess());
+
+        return expiryPlcFactoryStr.equals("") ? "Eternal" : 
expiryPlcFactoryStr;
+    }
+
+    /** @return {@code Yes} if group has expired entries, {@code No} 
otherwise. If {@code eagerTtl = true} returns 'Unknown'*/
+    public String hasEntriesPendingExpire() {

Review Comment:
   Can we shorten the method name? In SQL view it would be 
"HAS_ENTRIES_PENDING_EXPIRE" which is not very convinient.



##########
modules/core/src/main/java/org/apache/ignite/internal/managers/systemview/walker/CacheViewWalker.java:
##########
@@ -97,6 +97,7 @@ public class CacheViewWalker implements 
SystemViewRowAttributeWalker<CacheView>
         v.accept(59, "writeBehindFlushSize", int.class);
         v.accept(60, "writeBehindFlushThreadCount", int.class);
         v.accept(61, "writeSynchronizationMode", 
CacheWriteSynchronizationMode.class);
+        v.accept(62, "hasEntriesPendingExpire", String.class);

Review Comment:
   'Walker' classes shold not be changed manually. You should regenerate it 
using `SystemViewRowAttributeWalkerGenerator` (see javadoc for class).
   You forgot to change `count` method and fields order is incorrect.
   As far as I remember we have a tests checking full set of view columns. 
These tests should should be fixed to. Check team-city.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheGroupDescriptor.java:
##########
@@ -56,6 +56,10 @@ public class CacheGroupDescriptor {
     @GridToStringExclude
     private volatile CacheConfiguration<?, ?> cacheCfg;
 
+    /** */
+    @GridToStringExclude
+    private volatile GridCacheProcessor cacheProc;

Review Comment:
   `CacheGroupDescriptor` is some kind of DTO, which is distributed between 
nodes. Should not contain any node related classes (managers/processors/etc).



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/
+    @Test
+    public void testCacheViewExpiryPolicyWithPersist() throws Exception {
+        testCacheViewExpiryPolicy(true);
+    }
+
+    /** Tests work of {@link SystemView} for cache groups expiry policy info */
+    private void testCacheViewExpiryPolicy(boolean withPersistance) throws 
Exception {

Review Comment:
   `withPersistance` -> `withPersistence`



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";
+
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("create", 
expiryPlc.getExpiryForCreation());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("update", 
expiryPlc.getExpiryForUpdate());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("access", 
expiryPlc.getExpiryForAccess());
+
+        return expiryPlcFactoryStr.equals("") ? "Eternal" : 
expiryPlcFactoryStr;
+    }
+
+    /** @return {@code Yes} if group has expired entries, {@code No} 
otherwise. If {@code eagerTtl = true} returns 'Unknown'*/
+    public String hasEntriesPendingExpire() {
+        CacheGroupContext grpCtx = ctx.cache().cacheGroup(cache.groupId());
+
+        if (!cache.cacheConfiguration().isEagerTtl() || grpCtx == null || 
grpCtx.name() == null)

Review Comment:
   Why do we skip caches with `grpCtx.name() == null`?
   For `grpCtx == null` let's leave field empty or change the message. SInce 
when `isEagerTtl` is false - we can't check if expiring entries are exist, but 
when `grpCtx == null` - this mean node doesn't contain any cache group related 
data at all.



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/
+    @Test
+    public void testCacheViewExpiryPolicyWithPersist() throws Exception {
+        testCacheViewExpiryPolicy(true);
+    }
+
+    /** Tests work of {@link SystemView} for cache groups expiry policy info */
+    private void testCacheViewExpiryPolicy(boolean withPersistance) throws 
Exception {
+        try (IgniteEx g = !withPersistance ? startGrid() : 
startGrid(getConfiguration().setDataStorageConfiguration(
+            new DataStorageConfiguration().setDefaultDataRegionConfiguration(
+                new DataRegionConfiguration().setPersistenceEnabled(true)

Review Comment:
   Always tested with persistence



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/
+    @Test
+    public void testCacheViewExpiryPolicyWithPersist() throws Exception {
+        testCacheViewExpiryPolicy(true);
+    }
+
+    /** Tests work of {@link SystemView} for cache groups expiry policy info */
+    private void testCacheViewExpiryPolicy(boolean withPersistance) throws 
Exception {
+        try (IgniteEx g = !withPersistance ? startGrid() : 
startGrid(getConfiguration().setDataStorageConfiguration(
+            new DataStorageConfiguration().setDefaultDataRegionConfiguration(
+                new DataRegionConfiguration().setPersistenceEnabled(true)
+            )))) {
+
+            if (withPersistance)
+                g.cluster().state(ClusterState.ACTIVE);
+
+            String eternalCacheName = "eternalCache";
+            String createdCacheName = "createdCache";
+            String eagerTtlCacheName = "eagerTtlCache";
+            String withoutGrpCacheName = "withoutGrpCache";
+
+            CacheConfiguration<Long, Long> eternalCache = new 
CacheConfiguration<Long, Long>(eternalCacheName)
+                .setGroupName("group1")
+                .setExpiryPolicyFactory(EternalExpiryPolicy.factoryOf());
+
+            CacheConfiguration<Long, Long> createdCache = new 
CacheConfiguration<Long, Long>(createdCacheName)
+                .setGroupName("group2")
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            CacheConfiguration<Long, Long> eagerTtlCache = new 
CacheConfiguration<Long, Long>(eagerTtlCacheName)
+                .setGroupName("group2")
+                .setEagerTtl(false)
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            CacheConfiguration<Long, Long> withoutGrpCache = new 
CacheConfiguration<Long, Long>(withoutGrpCacheName)
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            g.createCache(eternalCache);
+            g.createCache(createdCache);
+            g.createCache(eagerTtlCache);
+            g.createCache(withoutGrpCache);
+
+            SystemView<CacheView> caches = 
g.context().systemView().view(CACHES_VIEW);
+
+            for (CacheView row : caches) {
+                switch (row.cacheName()) {
+                    case "eternalCache":
+                        assertTrue(row.hasEntriesPendingExpire().equals("No"));

Review Comment:
   Why not `assertEquals("No", row.hasEntriesPendingExpire())`?



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/
+    @Test
+    public void testCacheViewExpiryPolicyWithPersist() throws Exception {
+        testCacheViewExpiryPolicy(true);
+    }
+
+    /** Tests work of {@link SystemView} for cache groups expiry policy info */
+    private void testCacheViewExpiryPolicy(boolean withPersistance) throws 
Exception {
+        try (IgniteEx g = !withPersistance ? startGrid() : 
startGrid(getConfiguration().setDataStorageConfiguration(
+            new DataStorageConfiguration().setDefaultDataRegionConfiguration(
+                new DataRegionConfiguration().setPersistenceEnabled(true)
+            )))) {
+
+            if (withPersistance)
+                g.cluster().state(ClusterState.ACTIVE);
+
+            String eternalCacheName = "eternalCache";
+            String createdCacheName = "createdCache";
+            String eagerTtlCacheName = "eagerTtlCache";
+            String withoutGrpCacheName = "withoutGrpCache";
+
+            CacheConfiguration<Long, Long> eternalCache = new 
CacheConfiguration<Long, Long>(eternalCacheName)
+                .setGroupName("group1")
+                .setExpiryPolicyFactory(EternalExpiryPolicy.factoryOf());
+
+            CacheConfiguration<Long, Long> createdCache = new 
CacheConfiguration<Long, Long>(createdCacheName)
+                .setGroupName("group2")
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            CacheConfiguration<Long, Long> eagerTtlCache = new 
CacheConfiguration<Long, Long>(eagerTtlCacheName)
+                .setGroupName("group2")
+                .setEagerTtl(false)
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            CacheConfiguration<Long, Long> withoutGrpCache = new 
CacheConfiguration<Long, Long>(withoutGrpCacheName)
+                .setExpiryPolicyFactory(CreatedExpiryPolicy.factoryOf(new 
Duration(TimeUnit.MILLISECONDS, 500L)));
+
+            g.createCache(eternalCache);
+            g.createCache(createdCache);
+            g.createCache(eagerTtlCache);
+            g.createCache(withoutGrpCache);
+
+            SystemView<CacheView> caches = 
g.context().systemView().view(CACHES_VIEW);
+
+            for (CacheView row : caches) {
+                switch (row.cacheName()) {
+                    case "eternalCache":
+                        assertTrue(row.hasEntriesPendingExpire().equals("No"));
+
+                        g.cache(eternalCacheName).put(0, 0);
+
+                        assertTrue(row.hasEntriesPendingExpire().equals("No"));
+
+                        g.cache(eternalCacheName)
+                            .withExpiryPolicy(new CreatedExpiryPolicy(new 
Duration(TimeUnit.MILLISECONDS, 200L)))
+                            .put(1, 1);
+
+                        
assertTrue(row.hasEntriesPendingExpire().equals("Yes"));
+                        assertTrue(waitForCondition(() -> 
row.hasEntriesPendingExpire().equals("No"), getTestTimeout()));
+
+                        break;
+
+                    case "createdCache":
+                        assertTrue(row.hasEntriesPendingExpire().equals("No"));
+
+                        g.cache(createdCacheName).put(0, 0);
+
+                        
assertTrue(row.hasEntriesPendingExpire().equals("Yes"));
+                        assertTrue(waitForCondition(() -> 
row.hasEntriesPendingExpire().equals("No"), getTestTimeout()));
+
+                        g.cache(createdCacheName)
+                            .withExpiryPolicy(new ModifiedExpiryPolicy(new 
Duration(TimeUnit.MILLISECONDS, 200L)))
+                            .put(1, 1);
+
+                        
assertTrue(row.hasEntriesPendingExpire().equals("Yes"));
+                        assertTrue(waitForCondition(() -> 
row.hasEntriesPendingExpire().equals("No"), getTestTimeout()));
+
+                        g.cache(eagerTtlCacheName).put(2, 2);

Review Comment:
   Why we use `eagerTtlCacheName` here?



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/

Review Comment:
   Point and space at the end of sentence.



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";
+
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("create", 
expiryPlc.getExpiryForCreation());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("update", 
expiryPlc.getExpiryForUpdate());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("access", 
expiryPlc.getExpiryForAccess());
+
+        return expiryPlcFactoryStr.equals("") ? "Eternal" : 
expiryPlcFactoryStr;
+    }
+
+    /** @return {@code Yes} if group has expired entries, {@code No} 
otherwise. If {@code eagerTtl = true} returns 'Unknown'*/
+    public String hasEntriesPendingExpire() {
+        CacheGroupContext grpCtx = ctx.cache().cacheGroup(cache.groupId());
+
+        if (!cache.cacheConfiguration().isEagerTtl() || grpCtx == null || 
grpCtx.name() == null)
+            return "Unknown";
+
+        try {
+            return grpCtx.offheap().hasEntriesPendingExpire(cache.cacheId()) ? 
"Yes" : "No";
+        }
+        catch (IgniteCheckedException e) {
+            return e.getMessage();
+        }
+    }
+
+    /**
+     * Returns {@link Duration} String representation with description
+     * @param msg descriptive message for {@link Duration} instance
+     * @param duration @link Duration}
+     * @return {@link Duration} representation with descriptive message
+     */
+    public String durationToStringWithCustomMessage(String msg, Duration 
duration) {
+        if (duration == null || duration.getTimeUnit() == null || 
duration.getDurationAmount() == 0)
+            return "";
+
+        return "[" + msg + "=" + duration.getDurationAmount() + 
duration.getTimeUnit() + "]";

Review Comment:
   Space after `duration.getDurationAmount()` for better readability of duration



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";
+
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("create", 
expiryPlc.getExpiryForCreation());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("update", 
expiryPlc.getExpiryForUpdate());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("access", 
expiryPlc.getExpiryForAccess());
+
+        return expiryPlcFactoryStr.equals("") ? "Eternal" : 
expiryPlcFactoryStr;
+    }
+
+    /** @return {@code Yes} if group has expired entries, {@code No} 
otherwise. If {@code eagerTtl = true} returns 'Unknown'*/
+    public String hasEntriesPendingExpire() {
+        CacheGroupContext grpCtx = ctx.cache().cacheGroup(cache.groupId());
+
+        if (!cache.cacheConfiguration().isEagerTtl() || grpCtx == null || 
grpCtx.name() == null)
+            return "Unknown";
+
+        try {
+            return grpCtx.offheap().hasEntriesPendingExpire(cache.cacheId()) ? 
"Yes" : "No";
+        }
+        catch (IgniteCheckedException e) {
+            return e.getMessage();
+        }
+    }
+
+    /**
+     * Returns {@link Duration} String representation with description
+     * @param msg descriptive message for {@link Duration} instance
+     * @param duration @link Duration}
+     * @return {@link Duration} representation with descriptive message
+     */
+    public String durationToStringWithCustomMessage(String msg, Duration 
duration) {

Review Comment:
   `private static`
   Move method directly after the `expiryPolicyFactory` method (since it used 
only here) or to the end of the class.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheOffheapManagerImpl.java:
##########
@@ -1180,6 +1180,27 @@ private int expireInternal(
         return pendingEntries != null ? pendingEntries.size() : 0;
     }
 
+    /** {@inheritDoc} */
+    @Override public boolean hasEntriesPendingExpire() throws 
IgniteCheckedException {
+        return pendingEntries != null && !pendingEntries.isEmpty();
+    }
+
+    /** {@inheritDoc} */
+    @Override public boolean hasEntriesPendingExpire(int cacheId) throws 
IgniteCheckedException {
+        if (pendingEntries != null) {
+            PendingRow row = new PendingRow(cacheId);
+
+            GridCursor<PendingRow> cursor = pendingEntries.find(row, row, 
PendingEntriesTree.WITHOUT_KEY);

Review Comment:
   In case of not shared group we should use `exists`



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewSelfTest.java:
##########
@@ -259,6 +264,215 @@ public void testCacheGroupsView() throws Exception {
         }
     }
 
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
in-memory configuration */
+    @Test
+    public void testCacheViewExpiryPolicyWithInMemory() throws Exception {
+        testCacheViewExpiryPolicy(false);
+    }
+
+    /** Tests work of {@link SystemView} for cache expiry policy info with 
persist configuration*/
+    @Test
+    public void testCacheViewExpiryPolicyWithPersist() throws Exception {
+        testCacheViewExpiryPolicy(true);
+    }
+
+    /** Tests work of {@link SystemView} for cache groups expiry policy info */

Review Comment:
   Point at the end of sentence.



##########
modules/core/src/main/java/org/apache/ignite/spi/systemview/view/CacheView.java:
##########
@@ -331,7 +335,46 @@ public String cacheWriterFactory() {
 
     /** @see CacheConfiguration#getExpiryPolicyFactory() */
     public String expiryPolicyFactory() {
-        return 
toStringSafe(cache.cacheConfiguration().getExpiryPolicyFactory());
+        if (cache.cacheConfiguration().getExpiryPolicyFactory() == null)
+            return null;
+
+        ExpiryPolicy expiryPlc = 
(ExpiryPolicy)cache.cacheConfiguration().getExpiryPolicyFactory().create();
+
+        String expiryPlcFactoryStr = "";
+
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("create", 
expiryPlc.getExpiryForCreation());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("update", 
expiryPlc.getExpiryForUpdate());
+        expiryPlcFactoryStr += durationToStringWithCustomMessage("access", 
expiryPlc.getExpiryForAccess());
+
+        return expiryPlcFactoryStr.equals("") ? "Eternal" : 
expiryPlcFactoryStr;

Review Comment:
   Here we should output information about the factory in readable form (not 
only about instance), perhaps S.toString() on factory will be enough. 



##########
modules/core/src/test/java/org/apache/ignite/internal/metric/SystemViewCacheExpiryPolicyTest.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.ignite.internal.metric;
+
+import java.util.Arrays;
+import java.util.Collection;
+import javax.cache.expiry.Duration;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import 
org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicyFactory;
+import org.apache.ignite.spi.systemview.view.CacheView;
+import org.apache.ignite.spi.systemview.view.SystemView;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import static 
org.apache.ignite.internal.processors.cache.ClusterCachesInfo.CACHES_VIEW;
+
+/** Tests for {@link CacheView} expiry policy factory representation */
+@RunWith(Parameterized.class)
+public class SystemViewCacheExpiryPolicyTest extends GridCommonAbstractTest {
+    /**Create ttl {@link Duration}*/
+    @Parameterized.Parameter
+    public long create;
+
+    /** Access ttl {@link Duration}*/
+    @Parameterized.Parameter(1)
+    public long update;
+
+    /** Update ttl {@link Duration}*/
+    @Parameterized.Parameter(2)
+    public long access;
+
+    /** Anticipated {@link String} expiry policy factory representation*/
+    @Parameterized.Parameter(3)
+    public String actual;
+
+    /**
+     * @return Test parameters.
+     */
+    @Parameterized.Parameters(name = "create={0}, update={1}, access={2}, 
actual={3}")
+    public static Collection parameters() {
+        return Arrays.asList(new Object[][] {
+            {2, 4, 8, 
"[create=2MILLISECONDS][update=4MILLISECONDS][access=8MILLISECONDS]"},
+            {1, -2, -1, "[create=1MILLISECONDS]"},
+            {-1, 0, -1, "Eternal"},
+            {0, 1, -1, "[update=1MILLISECONDS]"}
+        });
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTestsStarted() throws Exception {
+        super.beforeTestsStarted();
+
+        cleanPersistenceDir();
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTestsStopped() throws Exception {
+        super.afterTestsStopped();
+
+        cleanPersistenceDir();
+    }
+
+    /**
+     * Test for {@link CacheView} expiry policy factory representation. The 
test initializes the {@link CacheConfiguration}
+     * with custom {@link PlatformExpiryPolicyFactory}. Given different ttl 
input, the test checks the {@link String}
+     * expiry policy factory outcome for {@link 
CacheView#expiryPolicyFactory()}.
+     */
+    @Test
+    public void testCacheViewExpiryPolicy() throws Exception {
+        try (IgniteEx g = startGrid()) {
+            CacheConfiguration<Integer, Integer> ccfg = new 
CacheConfiguration<>();
+            ccfg.setName("cache");
+            ccfg.setExpiryPolicyFactory(new 
PlatformExpiryPolicyFactory(create, update, access));

Review Comment:
   `SingletonFactory` should be tested too.



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to