nit0906 commented on code in PR #847:
URL: https://github.com/apache/jackrabbit-oak/pull/847#discussion_r1116564403
##########
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 - changes as part of this PR are already adding a hidden property
on the index :originalType in case this particular code marks it as disabled.
That case is already handled above and would not lead to a dangling ES index.
So basically - if :originalType is elasticsearch and type=disabled and this
code gets called from ES impl - then the ES index from remote will also get
deleted.
However if someone manually had marked the index as disabled - then of
course there's no way to determine the original index type (unless someone also
sets the :originalType property in that case). - This is the only case where
there would be a dangling index left.
Or were you suggesting something else ?
--
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]
