You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/25 11:52:59 UTC

[GitHub] [ignite] konstantins304 opened a new pull request #9044: IGNITE-13810 append expire policy info into log of started cache

konstantins304 opened a new pull request #9044:
URL: https://github.com/apache/ignite/pull/9044


   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] There is a single JIRA ticket related to the pull request. 
   - [ ] The web-link to the pull request is attached to the JIRA ticket.
   - [ ] The JIRA ticket has the _Patch Available_ state.
   - [ ] The pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
   - [ ] The pull request title is treated as the final commit message. 
   The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


-- 
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.

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



[GitHub] [ignite] ivandasch edited a comment on pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch edited a comment on pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#issuecomment-843963900


   I suppose that constantly running python tests doesn't help.
   
   If you look at logs, you will see NPE 
   ```
   [2021-05-19T11:17:57,828][ERROR][exchange-worker-#118][GridDhtPartitionsExchangeFuture] Failed to initialize cache(s) (will try to rollback) [exchId=GridDhtPartitionExchangeId [topVer=AffinityTopologyVersion [topVer=3, minorTopVer=8], discoEvt=DiscoveryCustomEvent [customMsg=DynamicCacheChangeBatch [id=3f923b38971-7e2b344a-a560-4e74-a44e-8798a26c073a, reqs=ArrayList [DynamicCacheChangeRequest [cacheName=expiry_cache, hasCfg=true, nodeId=b2f50d47-946d-49cd-82f4-a869a3b542bd, clientStartOnly=false, stop=false, destroy=false, disabledAfterStart=false]], exchangeActions=ExchangeActions [startCaches=[expiry_cache], stopCaches=null, startGrps=[expiry_cache], stopGrps=[], resetParts=null, stateChangeRequest=null], startCaches=false], affTopVer=AffinityTopologyVersion [topVer=3, minorTopVer=8], super=DiscoveryEvent [evtNode=TcpDiscoveryNode [id=b2f50d47-946d-49cd-82f4-a869a3b542bd, consistentId=srv_1, addrs=ArrayList [127.0.0.1], sockAddrs=HashSet [/127.0.0.1:48500], discPort=48500, order
 =1, intOrder=1, lastExchangeTime=1621412227458, loc=true, ver=2.11.0#20210519-sha1:00000000, isClient=false], topVer=3, msgTemplate=null, span=o.a.i.i.processors.tracing.NoopSpan@695cee68, nodeId8=b2f50d47, msg=null, type=DISCOVERY_CUSTOM_EVT, tstamp=1621412277807]], nodeId=b2f50d47, evt=DISCOVERY_CUSTOM_EVT], caches=[o.a.i.i.processors.cache.ExchangeActions$CacheGroupActionData@3b5d0f91]]
   java.lang.NullPointerException: null
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.buildExpirePolicyInfo(GridCacheProcessor.java:2410) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.onCacheStarted(GridCacheProcessor.java:2311) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.prepareCacheStart(GridCacheProcessor.java:1977) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.lambda$prepareStartCaches$55a0e703$1(GridCacheProcessor.java:1841) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.lambda$prepareStartCachesIfPossible$14(GridCacheProcessor.java:1811) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.prepareStartCaches(GridCacheProcessor.java:1838) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.prepareStartCachesIfPossible(GridCacheProcessor.java:1809) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.CacheAffinitySharedManager.processCacheStartRequests(CacheAffinitySharedManager.java:999) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.CacheAffinitySharedManager.onCacheChangeRequest(CacheAffinitySharedManager.java:885) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.distributed.dht.preloader.GridDhtPartitionsExchangeFuture.onCacheChangeRequest(GridDhtPartitionsExchangeFuture.java:1449) [classes/:?]
     at org.apache.ignite.internal.processors.cache.distributed.dht.preloader.GridDhtPartitionsExchangeFuture.init(GridDhtPartitionsExchangeFuture.java:947) [classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCachePartitionExchangeManager$ExchangeWorker.body0(GridCachePartitionExchangeManager.java:3379) [classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCachePartitionExchangeManager$ExchangeWorker.body(GridCachePartitionExchangeManager.java:3201) [classes/:?]
     at org.apache.ignite.internal.util.worker.GridWorker.run(GridWorker.java:120) [classes/:?]
     at java.lang.Thread.run(Thread.java:748) [?:1.8.0_261]
   ```
   
   And everything works ok on master :) I suppose that you patch breaks something


-- 
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.

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



[GitHub] [ignite] ivandasch commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r635127877



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -2385,12 +2388,30 @@ private void onCacheStarted(GridCacheContext cacheCtx) throws IgniteCheckedExcep
                 ", mode=" + cfg.getCacheMode() +
                 ", atomicity=" + cfg.getAtomicityMode() +
                 ", backups=" + cfg.getBackups() +
-                ", mvcc=" + cacheCtx.mvccEnabled() + ']');
+                ", mvcc=" + cacheCtx.mvccEnabled() +
+                (cacheCtx.expiry() != null ? ", " + buildExpirePolicyInfo(cacheCtx) : "") + ']');
         }
 
         return cacheCtx;
     }
 
+    /**
+     * Get formatted string with expire policy info.
+     *
+     * @param cacheCtx - grid cache context.
+     * @return formatted expire policy info.
+     */
+    private String buildExpirePolicyInfo(GridCacheContext cacheCtx) {
+        ExpiryPolicy expPlc = cacheCtx.expiry();
+
+        Duration dur = expPlc.getExpiryForCreation() != null ? expPlc.getExpiryForCreation() :

Review comment:
       Thin clients use `org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy`
   It contains field `org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy#DUR_UNCHANGED`
   If thin client pass it, thin client handler convert it to `null`. In tests one of parameter is `ExpiryPolicy()` with all fields `DUR_UNCHANGED`. And this case cause NPE in your code. 
   
   Thids is absolutely valid case, so as I suggested, just logs three durations, preferrably in seconds, I suppose (floating point number and prepend 's', something like `[create=0.1s, update=0.1s, access=0.1s]`)




-- 
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.

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



[GitHub] [ignite] zstan commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r694523958



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1546,6 +1551,36 @@ private long checkPoolStarvation(
                 EventType.EVT_NODE_JOINED, localNode());
 
         startTimer.finishGlobalStage("Await exchange");
+
+        if (log.isInfoEnabled())
+            ackExpPlcInfoByCaches();
+    }
+
+    /**
+     * Acks expiration policy info by caches.
+     */
+    private void ackExpPlcInfoByCaches() {
+        final String r = ctx.cache().context().cacheContexts().stream()
+                .map(cacheCtx -> {
+                    ExpiryPolicy expPlc = cacheCtx.expiry();
+                    if (expPlc == null || expPlc instanceof EternalExpiryPolicy) return null;
+
+                    Duration dur;
+                    if (expPlc.getExpiryForCreation() != null)
+                        dur = expPlc.getExpiryForCreation();
+                    else if (expPlc.getExpiryForUpdate() != null)
+                        dur = expPlc.getExpiryForUpdate();
+                    else
+                        dur = expPlc.getExpiryForAccess();
+
+                    if (dur == null || dur.getTimeUnit() == null) return null;
+
+                    return "{cache=" + cacheCtx.name() + ", duration=" + dur.getTimeUnit().toMillis(dur.getDurationAmount()) +

Review comment:
       why do you use "{" braces ? check other logs format style.




-- 
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-unsubscribe@ignite.apache.org

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



[GitHub] [ignite] ivandasch commented on pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#issuecomment-843963900


   I suppose that constantly running python tests doesn't help.
   
   If you look at logs, you will see NPE 
   ```
   [2021-05-19T11:17:57,828][ERROR][exchange-worker-#118][GridDhtPartitionsExchangeFuture] Failed to initialize cache(s) (will try to rollback) [exchId=GridDhtPartitionExchangeId [topVer=AffinityTopologyVersion [topVer=3, minorTopVer=8], discoEvt=DiscoveryCustomEvent [customMsg=DynamicCacheChangeBatch [id=3f923b38971-7e2b344a-a560-4e74-a44e-8798a26c073a, reqs=ArrayList [DynamicCacheChangeRequest [cacheName=expiry_cache, hasCfg=true, nodeId=b2f50d47-946d-49cd-82f4-a869a3b542bd, clientStartOnly=false, stop=false, destroy=false, disabledAfterStart=false]], exchangeActions=ExchangeActions [startCaches=[expiry_cache], stopCaches=null, startGrps=[expiry_cache], stopGrps=[], resetParts=null, stateChangeRequest=null], startCaches=false], affTopVer=AffinityTopologyVersion [topVer=3, minorTopVer=8], super=DiscoveryEvent [evtNode=TcpDiscoveryNode [id=b2f50d47-946d-49cd-82f4-a869a3b542bd, consistentId=srv_1, addrs=ArrayList [127.0.0.1], sockAddrs=HashSet [/127.0.0.1:48500], discPort=48500, order
 =1, intOrder=1, lastExchangeTime=1621412227458, loc=true, ver=2.11.0#20210519-sha1:00000000, isClient=false], topVer=3, msgTemplate=null, span=o.a.i.i.processors.tracing.NoopSpan@695cee68, nodeId8=b2f50d47, msg=null, type=DISCOVERY_CUSTOM_EVT, tstamp=1621412277807]], nodeId=b2f50d47, evt=DISCOVERY_CUSTOM_EVT], caches=[o.a.i.i.processors.cache.ExchangeActions$CacheGroupActionData@3b5d0f91]]
   java.lang.NullPointerException: null
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.buildExpirePolicyInfo(GridCacheProcessor.java:2410) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.onCacheStarted(GridCacheProcessor.java:2311) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.prepareCacheStart(GridCacheProcessor.java:1977) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.lambda$prepareStartCaches$55a0e703$1(GridCacheProcessor.java:1841) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.lambda$prepareStartCachesIfPossible$14(GridCacheProcessor.java:1811) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.prepareStartCaches(GridCacheProcessor.java:1838) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCacheProcessor.prepareStartCachesIfPossible(GridCacheProcessor.java:1809) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.CacheAffinitySharedManager.processCacheStartRequests(CacheAffinitySharedManager.java:999) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.CacheAffinitySharedManager.onCacheChangeRequest(CacheAffinitySharedManager.java:885) ~[classes/:?]
     at org.apache.ignite.internal.processors.cache.distributed.dht.preloader.GridDhtPartitionsExchangeFuture.onCacheChangeRequest(GridDhtPartitionsExchangeFuture.java:1449) [classes/:?]
     at org.apache.ignite.internal.processors.cache.distributed.dht.preloader.GridDhtPartitionsExchangeFuture.init(GridDhtPartitionsExchangeFuture.java:947) [classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCachePartitionExchangeManager$ExchangeWorker.body0(GridCachePartitionExchangeManager.java:3379) [classes/:?]
     at org.apache.ignite.internal.processors.cache.GridCachePartitionExchangeManager$ExchangeWorker.body(GridCachePartitionExchangeManager.java:3201) [classes/:?]
     at org.apache.ignite.internal.util.worker.GridWorker.run(GridWorker.java:120) [classes/:?]
     at java.lang.Thread.run(Thread.java:748) [?:1.8.0_261]
   ```


-- 
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.

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



[GitHub] [ignite] ivandasch commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r635109422



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -2385,12 +2388,30 @@ private void onCacheStarted(GridCacheContext cacheCtx) throws IgniteCheckedExcep
                 ", mode=" + cfg.getCacheMode() +
                 ", atomicity=" + cfg.getAtomicityMode() +
                 ", backups=" + cfg.getBackups() +
-                ", mvcc=" + cacheCtx.mvccEnabled() + ']');
+                ", mvcc=" + cacheCtx.mvccEnabled() +
+                (cacheCtx.expiry() != null ? ", " + buildExpirePolicyInfo(cacheCtx) : "") + ']');
         }
 
         return cacheCtx;
     }
 
+    /**
+     * Get formatted string with expire policy info.
+     *
+     * @param cacheCtx - grid cache context.
+     * @return formatted expire policy info.
+     */
+    private String buildExpirePolicyInfo(GridCacheContext cacheCtx) {
+        ExpiryPolicy expPlc = cacheCtx.expiry();
+
+        Duration dur = expPlc.getExpiryForCreation() != null ? expPlc.getExpiryForCreation() :

Review comment:
       Expiry policy can be null, FYI
   
   ```
   /**
    * @return Cache default {@link ExpiryPolicy}.
    */
   @Nullable public ExpiryPolicy expiry() {
       return expiryPlc;
   }
   ```




-- 
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.

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



[GitHub] [ignite] ivandasch commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r635127877



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -2385,12 +2388,30 @@ private void onCacheStarted(GridCacheContext cacheCtx) throws IgniteCheckedExcep
                 ", mode=" + cfg.getCacheMode() +
                 ", atomicity=" + cfg.getAtomicityMode() +
                 ", backups=" + cfg.getBackups() +
-                ", mvcc=" + cacheCtx.mvccEnabled() + ']');
+                ", mvcc=" + cacheCtx.mvccEnabled() +
+                (cacheCtx.expiry() != null ? ", " + buildExpirePolicyInfo(cacheCtx) : "") + ']');
         }
 
         return cacheCtx;
     }
 
+    /**
+     * Get formatted string with expire policy info.
+     *
+     * @param cacheCtx - grid cache context.
+     * @return formatted expire policy info.
+     */
+    private String buildExpirePolicyInfo(GridCacheContext cacheCtx) {
+        ExpiryPolicy expPlc = cacheCtx.expiry();
+
+        Duration dur = expPlc.getExpiryForCreation() != null ? expPlc.getExpiryForCreation() :

Review comment:
       Thin clients use `org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy`
   It contains field `org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy#DUR_UNCHANGED`
   If thin client pass it, thin client handler convert it to `null`. In tests one of parameter is `ExpiryPolicy()` with all fields `DUR_UNCHANGED`. And this case cause NPE in your code. 
   
   Thids is absolutely valid case, so as I suggested, just logs three durations, preferrably in seconds, I suppose (floating point number and prepend 's', something like `...., expirePolicy=[create=0.1s, update=0.1s, access=0.1s], isEagerTtl=false]`)




-- 
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.

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



[GitHub] [ignite] ivandasch commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r635113405



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -2385,12 +2388,30 @@ private void onCacheStarted(GridCacheContext cacheCtx) throws IgniteCheckedExcep
                 ", mode=" + cfg.getCacheMode() +
                 ", atomicity=" + cfg.getAtomicityMode() +
                 ", backups=" + cfg.getBackups() +
-                ", mvcc=" + cacheCtx.mvccEnabled() + ']');
+                ", mvcc=" + cacheCtx.mvccEnabled() +
+                (cacheCtx.expiry() != null ? ", " + buildExpirePolicyInfo(cacheCtx) : "") + ']');
         }
 
         return cacheCtx;
     }
 
+    /**
+     * Get formatted string with expire policy info.
+     *
+     * @param cacheCtx - grid cache context.
+     * @return formatted expire policy info.
+     */
+    private String buildExpirePolicyInfo(GridCacheContext cacheCtx) {
+        ExpiryPolicy expPlc = cacheCtx.expiry();
+
+        Duration dur = expPlc.getExpiryForCreation() != null ? expPlc.getExpiryForCreation() :

Review comment:
       Sorry, I suppose issue is not here.
   You should not log expiry policy like here.
   Expiry policy contains 3 fields and all of them can be used.
   
   So you must log all durations of expiry policy. I suggests log -1 if duration is null
   




-- 
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.

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



[GitHub] [ignite] zstan commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r694523342



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1546,6 +1551,36 @@ private long checkPoolStarvation(
                 EventType.EVT_NODE_JOINED, localNode());
 
         startTimer.finishGlobalStage("Await exchange");
+
+        if (log.isInfoEnabled())
+            ackExpPlcInfoByCaches();
+    }
+
+    /**
+     * Acks expiration policy info by caches.
+     */
+    private void ackExpPlcInfoByCaches() {
+        final String r = ctx.cache().context().cacheContexts().stream()
+                .map(cacheCtx -> {
+                    ExpiryPolicy expPlc = cacheCtx.expiry();
+                    if (expPlc == null || expPlc instanceof EternalExpiryPolicy) return null;
+
+                    Duration dur;
+                    if (expPlc.getExpiryForCreation() != null)
+                        dur = expPlc.getExpiryForCreation();
+                    else if (expPlc.getExpiryForUpdate() != null)
+                        dur = expPlc.getExpiryForUpdate();
+                    else
+                        dur = expPlc.getExpiryForAccess();
+
+                    if (dur == null || dur.getTimeUnit() == null) return null;
+
+                    return "{cache=" + cacheCtx.name() + ", duration=" + dur.getTimeUnit().toMillis(dur.getDurationAmount()) +
+                            "ms, isEagerTtl=" + cacheCtx.ttl().eagerTtlEnabled() + "}";
+                }).filter(s -> !Objects.isNull(s))

Review comment:
       Objects.notNull more accurate here




-- 
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-unsubscribe@ignite.apache.org

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



[GitHub] [ignite] zstan commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r694526278



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1546,6 +1551,36 @@ private long checkPoolStarvation(
                 EventType.EVT_NODE_JOINED, localNode());
 
         startTimer.finishGlobalStage("Await exchange");
+
+        if (log.isInfoEnabled())
+            ackExpPlcInfoByCaches();
+    }
+
+    /**
+     * Acks expiration policy info by caches.
+     */
+    private void ackExpPlcInfoByCaches() {

Review comment:
       func name confusing, change it for more informative.
   Also we need tests checks correct logging all cache expiration variations.




-- 
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-unsubscribe@ignite.apache.org

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



[GitHub] [ignite] zstan commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r694523119



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1546,6 +1551,36 @@ private long checkPoolStarvation(
                 EventType.EVT_NODE_JOINED, localNode());
 
         startTimer.finishGlobalStage("Await exchange");
+
+        if (log.isInfoEnabled())
+            ackExpPlcInfoByCaches();
+    }
+
+    /**
+     * Acks expiration policy info by caches.
+     */
+    private void ackExpPlcInfoByCaches() {
+        final String r = ctx.cache().context().cacheContexts().stream()
+                .map(cacheCtx -> {
+                    ExpiryPolicy expPlc = cacheCtx.expiry();
+                    if (expPlc == null || expPlc instanceof EternalExpiryPolicy) return null;
+
+                    Duration dur;
+                    if (expPlc.getExpiryForCreation() != null)
+                        dur = expPlc.getExpiryForCreation();
+                    else if (expPlc.getExpiryForUpdate() != null)
+                        dur = expPlc.getExpiryForUpdate();
+                    else
+                        dur = expPlc.getExpiryForAccess();
+
+                    if (dur == null || dur.getTimeUnit() == null) return null;
+
+                    return "{cache=" + cacheCtx.name() + ", duration=" + dur.getTimeUnit().toMillis(dur.getDurationAmount()) +
+                            "ms, isEagerTtl=" + cacheCtx.ttl().eagerTtlEnabled() + "}";
+                }).filter(s -> !Objects.isNull(s))
+                .collect(Collectors.joining(", "));
+
+        log.info(String.format("Expiration policy info for caches [%s]", r));

Review comment:
       do we need to print this message if previously return null and thus final String r = "" ?




-- 
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-unsubscribe@ignite.apache.org

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



[GitHub] [ignite] ivandasch commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r635109422



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -2385,12 +2388,30 @@ private void onCacheStarted(GridCacheContext cacheCtx) throws IgniteCheckedExcep
                 ", mode=" + cfg.getCacheMode() +
                 ", atomicity=" + cfg.getAtomicityMode() +
                 ", backups=" + cfg.getBackups() +
-                ", mvcc=" + cacheCtx.mvccEnabled() + ']');
+                ", mvcc=" + cacheCtx.mvccEnabled() +
+                (cacheCtx.expiry() != null ? ", " + buildExpirePolicyInfo(cacheCtx) : "") + ']');
         }
 
         return cacheCtx;
     }
 
+    /**
+     * Get formatted string with expire policy info.
+     *
+     * @param cacheCtx - grid cache context.
+     * @return formatted expire policy info.
+     */
+    private String buildExpirePolicyInfo(GridCacheContext cacheCtx) {
+        ExpiryPolicy expPlc = cacheCtx.expiry();
+
+        Duration dur = expPlc.getExpiryForCreation() != null ? expPlc.getExpiryForCreation() :

Review comment:
       Expiry policy can be null, FYI




-- 
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.

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



[GitHub] [ignite] zstan commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r694520078



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1546,6 +1551,36 @@ private long checkPoolStarvation(
                 EventType.EVT_NODE_JOINED, localNode());
 
         startTimer.finishGlobalStage("Await exchange");
+
+        if (log.isInfoEnabled())
+            ackExpPlcInfoByCaches();
+    }
+
+    /**
+     * Acks expiration policy info by caches.
+     */
+    private void ackExpPlcInfoByCaches() {
+        final String r = ctx.cache().context().cacheContexts().stream()
+                .map(cacheCtx -> {
+                    ExpiryPolicy expPlc = cacheCtx.expiry();
+                    if (expPlc == null || expPlc instanceof EternalExpiryPolicy) return null;
+
+                    Duration dur;
+                    if (expPlc.getExpiryForCreation() != null)
+                        dur = expPlc.getExpiryForCreation();
+                    else if (expPlc.getExpiryForUpdate() != null)
+                        dur = expPlc.getExpiryForUpdate();
+                    else
+                        dur = expPlc.getExpiryForAccess();
+
+                    if (dur == null || dur.getTimeUnit() == null) return null;

Review comment:
       return need to be on a new line




-- 
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-unsubscribe@ignite.apache.org

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



[GitHub] [ignite] zstan commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r694525457



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1546,6 +1551,36 @@ private long checkPoolStarvation(
                 EventType.EVT_NODE_JOINED, localNode());
 
         startTimer.finishGlobalStage("Await exchange");
+
+        if (log.isInfoEnabled())
+            ackExpPlcInfoByCaches();
+    }
+
+    /**
+     * Acks expiration policy info by caches.
+     */
+    private void ackExpPlcInfoByCaches() {
+        final String r = ctx.cache().context().cacheContexts().stream()
+                .map(cacheCtx -> {
+                    ExpiryPolicy expPlc = cacheCtx.expiry();
+                    if (expPlc == null || expPlc instanceof EternalExpiryPolicy) return null;

Review comment:
       return need to be on a new line




-- 
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-unsubscribe@ignite.apache.org

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



[GitHub] [ignite] ivandasch commented on a change in pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9044:
URL: https://github.com/apache/ignite/pull/9044#discussion_r635127877



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -2385,12 +2388,30 @@ private void onCacheStarted(GridCacheContext cacheCtx) throws IgniteCheckedExcep
                 ", mode=" + cfg.getCacheMode() +
                 ", atomicity=" + cfg.getAtomicityMode() +
                 ", backups=" + cfg.getBackups() +
-                ", mvcc=" + cacheCtx.mvccEnabled() + ']');
+                ", mvcc=" + cacheCtx.mvccEnabled() +
+                (cacheCtx.expiry() != null ? ", " + buildExpirePolicyInfo(cacheCtx) : "") + ']');
         }
 
         return cacheCtx;
     }
 
+    /**
+     * Get formatted string with expire policy info.
+     *
+     * @param cacheCtx - grid cache context.
+     * @return formatted expire policy info.
+     */
+    private String buildExpirePolicyInfo(GridCacheContext cacheCtx) {
+        ExpiryPolicy expPlc = cacheCtx.expiry();
+
+        Duration dur = expPlc.getExpiryForCreation() != null ? expPlc.getExpiryForCreation() :

Review comment:
       Thin clients use `org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy`
   It contains field `org.apache.ignite.internal.processors.platform.cache.expiry.PlatformExpiryPolicy#DUR_UNCHANGED`
   If thin client pass it, thin client handler convert it to `null`. In tests one of parameter is `ExpiryPolicy()` with all fields `DUR_UNCHANGED`. And this case cause NPE in your code




-- 
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.

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



[GitHub] [ignite] zstan closed pull request #9044: IGNITE-13810 append expire policy info into log of started cache

Posted by GitBox <gi...@apache.org>.
zstan closed pull request #9044:
URL: https://github.com/apache/ignite/pull/9044


   


-- 
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-unsubscribe@ignite.apache.org

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