You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by "jsinovassin (via GitHub)" <gi...@apache.org> on 2023/02/21 18:00:33 UTC

[GitHub] [unomi] jsinovassin opened a new pull request, #578: Unomi 739 update purge system

jsinovassin opened a new pull request, #578:
URL: https://github.com/apache/unomi/pull/578

   https://jira.jahia.org/browse/UNOMI-739


-- 
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: dev-unsubscribe@unomi.apache.org

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


[GitHub] [unomi] jkevan commented on a diff in pull request #578: Unomi 739 update purge system

Posted by "jkevan (via GitHub)" <gi...@apache.org>.
jkevan commented on code in PR #578:
URL: https://github.com/apache/unomi/pull/578#discussion_r1121359295


##########
api/src/main/java/org/apache/unomi/api/services/ProfileService.java:
##########
@@ -427,8 +426,14 @@ default Session loadSession(String sessionId) {
     void purgeProfiles(int inactiveNumberOfDays, int existsNumberOfDays);
 
     /**
-     * Purge (delete) monthly indices by removing old indices
-     * @param existsNumberOfMonths used to remove monthly indices older than this number of months
+     * Purge (delete) session items
+     * @param existsNumberOfDays used to remove monthly indices older than this number of days

Review Comment:
   Java doc not correct, still talking about monthly index



##########
api/src/main/java/org/apache/unomi/api/services/ProfileService.java:
##########
@@ -427,8 +426,14 @@ default Session loadSession(String sessionId) {
     void purgeProfiles(int inactiveNumberOfDays, int existsNumberOfDays);
 
     /**
-     * Purge (delete) monthly indices by removing old indices
-     * @param existsNumberOfMonths used to remove monthly indices older than this number of months
+     * Purge (delete) session items
+     * @param existsNumberOfDays used to remove monthly indices older than this number of days
+     */
+    void purgeSessionItems(int existsNumberOfDays);
+
+    /**
+     * Purge (delete) event items
+     * @param existsNumberOfDays used to remove monthly indices older than this number of days

Review Comment:
   Should we not deprecate: urgeMonthlyItems instead of removing it from the service interface ?



##########
services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java:
##########
@@ -367,25 +371,76 @@ public void purgeProfiles(int inactiveNumberOfDays, int existsNumberOfDays) {
     }
 
     @Override
-    public void purgeMonthlyItems(int existsNumberOfMonths) {
-        if (existsNumberOfMonths > 0) {
-            logger.info("Purging: Monthly items (sessions/events) created before {} months", existsNumberOfMonths);
-            persistenceService.purge(getMonth(-existsNumberOfMonths).getTime());
+    public void purgeSessionItems(int existsNumberOfDays) {
+        if (existsNumberOfDays > 0) {
+            ConditionType propertyConditionType = definitionsService.getConditionType("sessionPropertyCondition");
+            if (propertyConditionType == null) {
+                // definition service not yet fully instantiate
+                return;
+            }
+
+            Condition condition = new Condition(propertyConditionType);
+
+            logger.info("Purging: Session created since more than {} days", existsNumberOfDays);
+            condition.setParameter("propertyName", "timeStamp");
+            condition.setParameter("comparisonOperator", "lessThanOrEqualTo");
+            condition.setParameter("propertyValueDateExpr", "now-" + existsNumberOfDays + "d");
+
+            persistenceService.removeByQuery(condition, Session.class);
+            deleteEmptyRolloverIndex(Session.ITEM_TYPE);
+        }
+    }
+
+    @Override
+    public void purgeEventItems(int existsNumberOfDays) {
+        if (existsNumberOfDays > 0) {
+            ConditionType propertyConditionType = definitionsService.getConditionType("eventPropertyCondition");
+            if (propertyConditionType == null) {
+                // definition service not yet fully instantiate
+                return;
+            }
+
+            Condition condition = new Condition(propertyConditionType);
+
+            logger.info("Purging: Session created since more than {} days", existsNumberOfDays);
+            condition.setParameter("propertyName", "timeStamp");
+            condition.setParameter("comparisonOperator", "lessThanOrEqualTo");
+            condition.setParameter("propertyValueDateExpr", "now-" + existsNumberOfDays + "d");
+
+            persistenceService.removeByQuery(condition, Event.class);
+            deleteEmptyRolloverIndex(Event.ITEM_TYPE);
+        }
+    }
+
+    public void deleteEmptyRolloverIndex(String indexName) {
+        TreeMap<String, Long> countsPerIndex = new TreeMap<>(persistenceService.docCountPerIndex(indexName));
+        if (countsPerIndex.size() >= 1) {
+            // do not check the last index, because it's the one used to write documents
+            countsPerIndex.pollLastEntry();
+            countsPerIndex.forEach((index, count) -> {
+                if (count == 0) {
+                    persistenceService.removeIndex(index, false);
+                }
+            });
         }
     }
 
     private void initializePurge() {
         logger.info("Purge: Initializing");
 
-        if (purgeProfileInactiveTime > 0 || purgeProfileExistTime > 0 || purgeSessionsAndEventsTime > 0) {
+        if (purgeProfileInactiveTime > 0 || purgeProfileExistTime > 0 || purgeSessionsAndEventsTime > 0 || purgeSessionExistTime > 0 || purgeEventExistTime > 0) {

Review Comment:
   For me purgeSessionsAndEventsTime is not necessary anymore since it is used to initialize the others in case the new one are not defined.



##########
package/src/main/resources/etc/custom.system.properties:
##########
@@ -164,6 +164,10 @@ org.apache.unomi.profile.purge.inactiveTime=${env:UNOMI_PROFILE_PURGE_INACTIVETI
 org.apache.unomi.profile.purge.existTime=${env:UNOMI_PROFILE_PURGE_EXISTTIME:--1}
 # Purge all monthly indexes (sessions/events) that have been created for a specific number of months
 org.apache.unomi.monthly.index.purge.existTime=${env:UNOMI_MONTHLY_INDEX_PURGE_EXISTTIME:-12}
+# Purge sessions that have been created for a specific number of days
+org.apache.unomi.sessions.purge.existTime=${env:UNOMI_SESSIONS_PURGE_EXISTTIME:-}

Review Comment:
   Could be nice to add some deprecation message on the old one in the conf file.



##########
services/src/main/java/org/apache/unomi/services/impl/profiles/ProfileServiceImpl.java:
##########
@@ -367,25 +371,76 @@ public void purgeProfiles(int inactiveNumberOfDays, int existsNumberOfDays) {
     }
 
     @Override
-    public void purgeMonthlyItems(int existsNumberOfMonths) {
-        if (existsNumberOfMonths > 0) {
-            logger.info("Purging: Monthly items (sessions/events) created before {} months", existsNumberOfMonths);
-            persistenceService.purge(getMonth(-existsNumberOfMonths).getTime());
+    public void purgeSessionItems(int existsNumberOfDays) {
+        if (existsNumberOfDays > 0) {
+            ConditionType propertyConditionType = definitionsService.getConditionType("sessionPropertyCondition");
+            if (propertyConditionType == null) {
+                // definition service not yet fully instantiate
+                return;
+            }
+
+            Condition condition = new Condition(propertyConditionType);
+
+            logger.info("Purging: Session created since more than {} days", existsNumberOfDays);
+            condition.setParameter("propertyName", "timeStamp");
+            condition.setParameter("comparisonOperator", "lessThanOrEqualTo");
+            condition.setParameter("propertyValueDateExpr", "now-" + existsNumberOfDays + "d");
+
+            persistenceService.removeByQuery(condition, Session.class);
+            deleteEmptyRolloverIndex(Session.ITEM_TYPE);
+        }
+    }
+
+    @Override
+    public void purgeEventItems(int existsNumberOfDays) {
+        if (existsNumberOfDays > 0) {
+            ConditionType propertyConditionType = definitionsService.getConditionType("eventPropertyCondition");
+            if (propertyConditionType == null) {
+                // definition service not yet fully instantiate
+                return;
+            }
+
+            Condition condition = new Condition(propertyConditionType);
+
+            logger.info("Purging: Session created since more than {} days", existsNumberOfDays);
+            condition.setParameter("propertyName", "timeStamp");
+            condition.setParameter("comparisonOperator", "lessThanOrEqualTo");
+            condition.setParameter("propertyValueDateExpr", "now-" + existsNumberOfDays + "d");
+
+            persistenceService.removeByQuery(condition, Event.class);
+            deleteEmptyRolloverIndex(Event.ITEM_TYPE);
+        }
+    }

Review Comment:
   The code look quiet similar could we create a generic method for this ?



##########
itests/src/test/java/org/apache/unomi/itests/ProfileServiceIT.java:
##########
@@ -372,30 +372,36 @@ public void testMonthlyIndicesPurge() throws Exception {
                 (count) -> count == (450 + originalEventsCount), 1000, 100);
 
         // Should have no effect
-        profileService.purgeMonthlyItems(0);
+        profileService.purgeSessionItems(0);
         keepTrying("Sessions number should be 450", () -> persistenceService.getAllItemsCount(Session.ITEM_TYPE),
                 (count) -> count == (450 + originalSessionsCount), 1000, 100);
+        profileService.purgeEventItems(0);
         keepTrying("Events number should be 450", () -> persistenceService.getAllItemsCount(Event.ITEM_TYPE),
                 (count) -> count == (450 + originalEventsCount), 1000, 100);
 
         // Should have no effect there is no monthly items older than 40 months
-        profileService.purgeMonthlyItems(40);
+        profileService.purgeSessionItems(40);

Review Comment:
   should not it be 40x30 ? because 40 was for month in previous test.



-- 
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: dev-unsubscribe@unomi.apache.org

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


[GitHub] [unomi] jsinovassin merged pull request #578: Unomi 739 update purge system

Posted by "jsinovassin (via GitHub)" <gi...@apache.org>.
jsinovassin merged PR #578:
URL: https://github.com/apache/unomi/pull/578


-- 
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: dev-unsubscribe@unomi.apache.org

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