You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/03/16 00:17:09 UTC

[GitHub] [jackrabbit-oak] FrancoisZhang opened a new pull request #520: GRANITE-38542 Adding cronjob for call cleanup index command which imp…

FrancoisZhang opened a new pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520


   …lemented in oak-run: adjust APIs and adding logs


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

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



[GitHub] [jackrabbit-oak] FrancoisZhang commented on a change in pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
FrancoisZhang commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r829474833



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/run/PurgeOldIndexVersionCommand.java
##########
@@ -17,17 +17,24 @@
 
 package org.apache.jackrabbit.oak.run;
 
-import joptsimple.OptionParser;
-import joptsimple.OptionSet;
-import joptsimple.OptionSpec;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.jackrabbit.oak.indexversion.PurgeOldIndexVersion;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixture;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixtureProvider;
 import org.apache.jackrabbit.oak.run.cli.Options;
 import org.apache.jackrabbit.oak.run.commons.Command;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.util.List;
-import java.util.concurrent.TimeUnit;
+import joptsimple.OptionParser;
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 
 public class PurgeOldIndexVersionCommand implements Command {
+    private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersionCommand.class);
+
     private long threshold;
     private List<String> indexPaths;
     private long DEFAULT_PURGE_THRESHOLD = TimeUnit.DAYS.toMillis(5); // 5 days in millis

Review comment:
       @fabriziofortino true, i's a legacy I didn't notice that. I will refine it together in next PR. seems need more logging 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: dev-unsubscribe@jackrabbit.apache.org

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



[GitHub] [jackrabbit-oak] thomasmueller merged pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
thomasmueller merged pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520


   


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

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



[GitHub] [jackrabbit-oak] reschke commented on pull request #520: GRANITE-38542 Adding cronjob for call cleanup index command which imp…

Posted by GitBox <gi...@apache.org>.
reschke commented on pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#issuecomment-1068769034


   Please refer to *OAK* Jira tickets in commit messages and PRs.


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

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



[GitHub] [jackrabbit-oak] thomasmueller commented on a change in pull request #520: GRANITE-38542 Adding cronjob for call cleanup index command which imp…

Posted by GitBox <gi...@apache.org>.
thomasmueller commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r827865527



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java
##########
@@ -38,43 +43,59 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class PurgeOldIndexVersion {
     private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersion.class);
 
-    public void execute(Options opts, long purgeThresholdMillis, List<String> indexPaths) throws Exception {
-        boolean isReadWriteRepository = opts.getCommonOpts().isReadWrite();
+    /**
+     * Execute purging index based on the index version naming and last time index time
+     *
+     * @param nodeStore             the node store
+     * @param isReadWriteRepository bool to indicate if it's read write repository, if yes, the purge index will not execute
+     * @param purgeThresholdMillis  the threshold of time length since last time index time to determine, will purge if exceed that
+     * @param indexPaths            the index path or parent path
+     *
+     * @throws IOException
+     * @throws CommitFailedException
+     */
+    public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
         if (!isReadWriteRepository) {
-            LOG.info("Repository connected in read-only mode. Use '--read-write' for write operations");
+            LOG.info("Repository is opened in read-only mode");

Review comment:
       I prefer the message above (with --read-write)

##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java
##########
@@ -38,43 +43,59 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class PurgeOldIndexVersion {
     private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersion.class);
 
-    public void execute(Options opts, long purgeThresholdMillis, List<String> indexPaths) throws Exception {
-        boolean isReadWriteRepository = opts.getCommonOpts().isReadWrite();
+    /**
+     * Execute purging index based on the index version naming and last time index time
+     *
+     * @param nodeStore             the node store
+     * @param isReadWriteRepository bool to indicate if it's read write repository, if yes, the purge index will not execute
+     * @param purgeThresholdMillis  the threshold of time length since last time index time to determine, will purge if exceed that
+     * @param indexPaths            the index path or parent path
+     *
+     * @throws IOException
+     * @throws CommitFailedException
+     */
+    public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
         if (!isReadWriteRepository) {
-            LOG.info("Repository connected in read-only mode. Use '--read-write' for write operations");
+            LOG.info("Repository is opened in read-only mode");
+            return;

Review comment:
       The old code allowed to just list the indexes that can be removed. Now this feature is gone... Why? I would prefer to keep it (it looks like it's quite easy to keep it).

##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java
##########
@@ -38,43 +43,59 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class PurgeOldIndexVersion {
     private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersion.class);
 
-    public void execute(Options opts, long purgeThresholdMillis, List<String> indexPaths) throws Exception {
-        boolean isReadWriteRepository = opts.getCommonOpts().isReadWrite();
+    /**
+     * Execute purging index based on the index version naming and last time index time
+     *
+     * @param nodeStore             the node store
+     * @param isReadWriteRepository bool to indicate if it's read write repository, if yes, the purge index will not execute
+     * @param purgeThresholdMillis  the threshold of time length since last time index time to determine, will purge if exceed that
+     * @param indexPaths            the index path or parent path
+     *
+     * @throws IOException
+     * @throws CommitFailedException
+     */
+    public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
         if (!isReadWriteRepository) {
-            LOG.info("Repository connected in read-only mode. Use '--read-write' for write operations");
+            LOG.info("Repository is opened in read-only mode");
+            return;
         }
-        try (NodeStoreFixture fixture = NodeStoreFixtureProvider.create(opts)) {
-            NodeStore nodeStore = fixture.getStore();
-            List<String> sanitisedIndexPaths = sanitiseUserIndexPaths(indexPaths);
-            Set<String> indexPathSet = filterIndexPaths(getRepositoryIndexPaths(nodeStore), sanitisedIndexPaths);
-            Map<String, Set<String>> segregateIndexes = segregateIndexes(indexPathSet);
-            for (Map.Entry<String, Set<String>> entry : segregateIndexes.entrySet()) {
-                String baseIndexPath = entry.getKey();
-                String parentPath = PathUtils.getParentPath(entry.getKey());
-                List<IndexName> indexNameObjectList = getIndexNameObjectList(entry.getValue());
-                NodeState indexDefParentNode = NodeStateUtils.getNode(nodeStore.getRoot(),
-                        parentPath);
-                List<IndexVersionOperation> toDeleteIndexNameObjectList =
-                        IndexVersionOperation.generateIndexVersionOperationList(indexDefParentNode, indexNameObjectList, purgeThresholdMillis);
-                if (isReadWriteRepository && !toDeleteIndexNameObjectList.isEmpty()) {
-                    purgeOldIndexVersion(nodeStore, toDeleteIndexNameObjectList);
-                } else {
-                    LOG.info("Repository is opened in read-only mode: IndexOperations" +
-                            " for index at path {} are : {}", baseIndexPath, toDeleteIndexNameObjectList);
-                }
+        List<IndexVersionOperation> purgeIndexList = getPurgeIndexes(nodeStore, purgeThresholdMillis, indexPaths);
+        if (!purgeIndexList.isEmpty()) {
+            LOG.info("Found indexes for purging: '{}'", purgeIndexList);
+            long purgeStart = System.currentTimeMillis();
+            purgeOldIndexVersion(nodeStore, purgeIndexList);
+            LOG.info("Index purging done, took '{}' ms", (System.currentTimeMillis() - purgeStart));
+        } else {
+            LOG.info("No indexes are found to be purged");
+        }
+    }
+
+    public List<IndexVersionOperation> getPurgeIndexes(NodeStore nodeStore, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
+        List<IndexVersionOperation> purgeIndexList = new ArrayList<>();
+        LOG.info("Start execute purge index over node type '{}' over index paths '{}'", nodeStore.getClass().getSimpleName(), indexPaths);

Review comment:
       I would log "Getting indexes to purge ..." as in this method we don't purge.




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

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



[GitHub] [jackrabbit-oak] fabriziofortino commented on a change in pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
fabriziofortino commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r829186015



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/run/PurgeOldIndexVersionCommand.java
##########
@@ -17,17 +17,24 @@
 
 package org.apache.jackrabbit.oak.run;
 
-import joptsimple.OptionParser;
-import joptsimple.OptionSet;
-import joptsimple.OptionSpec;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.jackrabbit.oak.indexversion.PurgeOldIndexVersion;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixture;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixtureProvider;
 import org.apache.jackrabbit.oak.run.cli.Options;
 import org.apache.jackrabbit.oak.run.commons.Command;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.util.List;
-import java.util.concurrent.TimeUnit;
+import joptsimple.OptionParser;
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 
 public class PurgeOldIndexVersionCommand implements Command {
+    private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersionCommand.class);
+
     private long threshold;
     private List<String> indexPaths;
     private long DEFAULT_PURGE_THRESHOLD = TimeUnit.DAYS.toMillis(5); // 5 days in millis

Review comment:
       this should be `final`




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

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



[GitHub] [jackrabbit-oak] FrancoisZhang commented on a change in pull request #520: GRANITE-38542 Adding cronjob for call cleanup index command which imp…

Posted by GitBox <gi...@apache.org>.
FrancoisZhang commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r828225335



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java
##########
@@ -38,43 +43,59 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class PurgeOldIndexVersion {
     private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersion.class);
 
-    public void execute(Options opts, long purgeThresholdMillis, List<String> indexPaths) throws Exception {
-        boolean isReadWriteRepository = opts.getCommonOpts().isReadWrite();
+    /**
+     * Execute purging index based on the index version naming and last time index time
+     *
+     * @param nodeStore             the node store
+     * @param isReadWriteRepository bool to indicate if it's read write repository, if yes, the purge index will not execute
+     * @param purgeThresholdMillis  the threshold of time length since last time index time to determine, will purge if exceed that
+     * @param indexPaths            the index path or parent path
+     *
+     * @throws IOException
+     * @throws CommitFailedException
+     */
+    public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
         if (!isReadWriteRepository) {
-            LOG.info("Repository connected in read-only mode. Use '--read-write' for write operations");
+            LOG.info("Repository is opened in read-only mode");

Review comment:
       @thomasmueller thanks, the reason I adjust it is the message fit for the case from oak run command line, but doesn't apply for the case if call it from tools image. Let me add it back, but bit earlier over PurgeOldIndexVersionCommand class




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

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



[GitHub] [jackrabbit-oak] thomasmueller merged pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
thomasmueller merged pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520


   


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

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



[GitHub] [jackrabbit-oak] FrancoisZhang commented on a change in pull request #520: GRANITE-38542 Adding cronjob for call cleanup index command which imp…

Posted by GitBox <gi...@apache.org>.
FrancoisZhang commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r828228769



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java
##########
@@ -38,43 +43,59 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class PurgeOldIndexVersion {
     private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersion.class);
 
-    public void execute(Options opts, long purgeThresholdMillis, List<String> indexPaths) throws Exception {
-        boolean isReadWriteRepository = opts.getCommonOpts().isReadWrite();
+    /**
+     * Execute purging index based on the index version naming and last time index time
+     *
+     * @param nodeStore             the node store
+     * @param isReadWriteRepository bool to indicate if it's read write repository, if yes, the purge index will not execute
+     * @param purgeThresholdMillis  the threshold of time length since last time index time to determine, will purge if exceed that
+     * @param indexPaths            the index path or parent path
+     *
+     * @throws IOException
+     * @throws CommitFailedException
+     */
+    public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
         if (!isReadWriteRepository) {
-            LOG.info("Repository connected in read-only mode. Use '--read-write' for write operations");
+            LOG.info("Repository is opened in read-only mode");
+            return;

Review comment:
       @thomasmueller yes, getting the list index is done in getPurgeIndexes and it will work when call from tools image. yes, I think makes sense to also list the indexes in oak run command line. will fix it.




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

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



[GitHub] [jackrabbit-oak] FrancoisZhang commented on pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
FrancoisZhang commented on pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#issuecomment-1069562855


   @reschke sorry for that, just created an OAK ticket and fix the message in PR
   
   @thomasmueller I have refined the commit over your code review, thanks!


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

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



[GitHub] [jackrabbit-oak] FrancoisZhang commented on a change in pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
FrancoisZhang commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r829474833



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/run/PurgeOldIndexVersionCommand.java
##########
@@ -17,17 +17,24 @@
 
 package org.apache.jackrabbit.oak.run;
 
-import joptsimple.OptionParser;
-import joptsimple.OptionSet;
-import joptsimple.OptionSpec;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.jackrabbit.oak.indexversion.PurgeOldIndexVersion;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixture;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixtureProvider;
 import org.apache.jackrabbit.oak.run.cli.Options;
 import org.apache.jackrabbit.oak.run.commons.Command;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.util.List;
-import java.util.concurrent.TimeUnit;
+import joptsimple.OptionParser;
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 
 public class PurgeOldIndexVersionCommand implements Command {
+    private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersionCommand.class);
+
     private long threshold;
     private List<String> indexPaths;
     private long DEFAULT_PURGE_THRESHOLD = TimeUnit.DAYS.toMillis(5); // 5 days in millis

Review comment:
       @fabriziofortino true, i's a legacy I didn't notice that. I will refine it together in next PR. seems need more logging 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: dev-unsubscribe@jackrabbit.apache.org

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



[GitHub] [jackrabbit-oak] fabriziofortino commented on a change in pull request #520: OAK-9726 Improve index purge old version commands logs

Posted by GitBox <gi...@apache.org>.
fabriziofortino commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r829186015



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/run/PurgeOldIndexVersionCommand.java
##########
@@ -17,17 +17,24 @@
 
 package org.apache.jackrabbit.oak.run;
 
-import joptsimple.OptionParser;
-import joptsimple.OptionSet;
-import joptsimple.OptionSpec;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.jackrabbit.oak.indexversion.PurgeOldIndexVersion;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixture;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixtureProvider;
 import org.apache.jackrabbit.oak.run.cli.Options;
 import org.apache.jackrabbit.oak.run.commons.Command;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.util.List;
-import java.util.concurrent.TimeUnit;
+import joptsimple.OptionParser;
+import joptsimple.OptionSet;
+import joptsimple.OptionSpec;
 
 public class PurgeOldIndexVersionCommand implements Command {
+    private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersionCommand.class);
+
     private long threshold;
     private List<String> indexPaths;
     private long DEFAULT_PURGE_THRESHOLD = TimeUnit.DAYS.toMillis(5); // 5 days in millis

Review comment:
       this should be `final`




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

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



[GitHub] [jackrabbit-oak] FrancoisZhang commented on a change in pull request #520: GRANITE-38542 Adding cronjob for call cleanup index command which imp…

Posted by GitBox <gi...@apache.org>.
FrancoisZhang commented on a change in pull request #520:
URL: https://github.com/apache/jackrabbit-oak/pull/520#discussion_r828226769



##########
File path: oak-run/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java
##########
@@ -38,43 +43,59 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class PurgeOldIndexVersion {
     private static final Logger LOG = LoggerFactory.getLogger(PurgeOldIndexVersion.class);
 
-    public void execute(Options opts, long purgeThresholdMillis, List<String> indexPaths) throws Exception {
-        boolean isReadWriteRepository = opts.getCommonOpts().isReadWrite();
+    /**
+     * Execute purging index based on the index version naming and last time index time
+     *
+     * @param nodeStore             the node store
+     * @param isReadWriteRepository bool to indicate if it's read write repository, if yes, the purge index will not execute
+     * @param purgeThresholdMillis  the threshold of time length since last time index time to determine, will purge if exceed that
+     * @param indexPaths            the index path or parent path
+     *
+     * @throws IOException
+     * @throws CommitFailedException
+     */
+    public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
         if (!isReadWriteRepository) {
-            LOG.info("Repository connected in read-only mode. Use '--read-write' for write operations");
+            LOG.info("Repository is opened in read-only mode");
+            return;
         }
-        try (NodeStoreFixture fixture = NodeStoreFixtureProvider.create(opts)) {
-            NodeStore nodeStore = fixture.getStore();
-            List<String> sanitisedIndexPaths = sanitiseUserIndexPaths(indexPaths);
-            Set<String> indexPathSet = filterIndexPaths(getRepositoryIndexPaths(nodeStore), sanitisedIndexPaths);
-            Map<String, Set<String>> segregateIndexes = segregateIndexes(indexPathSet);
-            for (Map.Entry<String, Set<String>> entry : segregateIndexes.entrySet()) {
-                String baseIndexPath = entry.getKey();
-                String parentPath = PathUtils.getParentPath(entry.getKey());
-                List<IndexName> indexNameObjectList = getIndexNameObjectList(entry.getValue());
-                NodeState indexDefParentNode = NodeStateUtils.getNode(nodeStore.getRoot(),
-                        parentPath);
-                List<IndexVersionOperation> toDeleteIndexNameObjectList =
-                        IndexVersionOperation.generateIndexVersionOperationList(indexDefParentNode, indexNameObjectList, purgeThresholdMillis);
-                if (isReadWriteRepository && !toDeleteIndexNameObjectList.isEmpty()) {
-                    purgeOldIndexVersion(nodeStore, toDeleteIndexNameObjectList);
-                } else {
-                    LOG.info("Repository is opened in read-only mode: IndexOperations" +
-                            " for index at path {} are : {}", baseIndexPath, toDeleteIndexNameObjectList);
-                }
+        List<IndexVersionOperation> purgeIndexList = getPurgeIndexes(nodeStore, purgeThresholdMillis, indexPaths);
+        if (!purgeIndexList.isEmpty()) {
+            LOG.info("Found indexes for purging: '{}'", purgeIndexList);
+            long purgeStart = System.currentTimeMillis();
+            purgeOldIndexVersion(nodeStore, purgeIndexList);
+            LOG.info("Index purging done, took '{}' ms", (System.currentTimeMillis() - purgeStart));
+        } else {
+            LOG.info("No indexes are found to be purged");
+        }
+    }
+
+    public List<IndexVersionOperation> getPurgeIndexes(NodeStore nodeStore, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
+        List<IndexVersionOperation> purgeIndexList = new ArrayList<>();
+        LOG.info("Start execute purge index over node type '{}' over index paths '{}'", nodeStore.getClass().getSimpleName(), indexPaths);

Review comment:
       @thomasmueller thanks, makes sense, will change it.




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

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