nit0906 commented on code in PR #847:
URL: https://github.com/apache/jackrabbit-oak/pull/847#discussion_r1118320215
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/indexversion/PurgeOldIndexVersion.java:
##########
@@ -166,13 +175,38 @@ private Iterable<String>
getRepositoryIndexPaths(NodeStore store) throws CommitF
* @param commandlineIndexPaths indexpaths provided by user
* @return returns set of indexpaths which are to be considered for purging
*/
- private Set<String> filterIndexPaths(Iterable<String>
repositoryIndexPaths, List<String> commandlineIndexPaths) {
+ private Set<String> filterIndexPaths(NodeStore store, Iterable<String>
repositoryIndexPaths, List<String> commandlineIndexPaths) {
Set<String> filteredIndexPaths = new HashSet<>();
+ NodeState root = store.getRoot();
+ NodeBuilder rootBuilder = root.builder();
for (String commandlineIndexPath : commandlineIndexPaths) {
for (String repositoryIndexPath : repositoryIndexPaths) {
if
(PurgeOldVersionUtils.isIndexChildNode(commandlineIndexPath,
repositoryIndexPath)
||
PurgeOldVersionUtils.isBaseIndexEqual(commandlineIndexPath,
repositoryIndexPath)) {
- filteredIndexPaths.add(repositoryIndexPath);
+ NodeBuilder nodeBuilder =
PurgeOldVersionUtils.getNode(rootBuilder, repositoryIndexPath);
+ if (nodeBuilder.exists()) {
+ String type =
nodeBuilder.getProperty(TYPE_PROPERTY_NAME).getValue(Type.STRING);
+ // If type = disabled. There can be 2 cases -
+ // 1. It was marked as disabled by this code , in
which case the original type would be moved to orig_type property
+ // 2. It was marked disabled by someone else.
+
+ // Skip the run if (type = disabled and orig_type is
present and not equal to PurgeOldIndexVersion's impl's index type)
+ // OR
+ // (type != disabled and type is not equal to
PurgeOldIndexVersion's impl's index type)
+ if ((TYPE_DISABLED.equals(type) &&
nodeBuilder.getProperty(ORIGINAL_TYPE_PROPERTY_NAME) != null &&
!getIndexType().equals(nodeBuilder.getProperty(ORIGINAL_TYPE_PROPERTY_NAME).getValue(Type.STRING)))
||
+ (!TYPE_DISABLED.equals(type) &&
!getIndexType().equals(type))) {
+ continue;
+ }
+
+ // If type = disabled and orig_type is not set - this
means someone manually marked the index as disabled
+ // We can't determine the original index
implementation in this case - so we let it run for whichever
PurgeOldIndexVersion's impl calls it first
+ // Two scenarios to note here -
+ // 1. ElasticPurgeOldIndexVersion calls this for a
lucene index (deletes the index from oak repo - which we want)
+ // and then in postDeleteOp tries to delete a non
existent remote elastic index - that particular call will simply fail.
+ // 2. LucenePurgeOldIndexVersion calls this for an
elastic index (deletes the index from oak repo)
+ // and then does a NOOP in postDeleteOp - effectively
we are left with a dangling elastic remote index in this case.
+ filteredIndexPaths.add(repositoryIndexPath);
+ }
Review Comment:
@tihom88 - made the changes to actually skip disabled indexes in case of
type=disabled and :originalType prop not set. This would basically exclude
manually disabled indexes from purge automated logic. Seems a bit more clean
this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
