Skip to content
Permalink
Browse files
Merge pull request #562 from FrancoisZhang/OAK-9760
OAK-9760 Oak run index purge command active index check is in correct
  • Loading branch information
thomasmueller committed May 10, 2022
2 parents c25e78d + 0b97afe commit 5ac5b177cba9f4e40641d69fed986cd420b6a17b
Showing 4 changed files with 118 additions and 43 deletions.
@@ -28,6 +28,7 @@
import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition;
import org.apache.jackrabbit.oak.plugins.index.search.spi.query.IndexName;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@@ -63,13 +64,18 @@ public String toString() {
}

/**
* @param indexDefParentNode NodeState of parent of baseIndex
* Generate list of index version operation over a list of indexes have same index base.
*
* @param rootNode NodeState of root
* @param parentPath parent path of baseIndex
* @param indexNameObjectList This is a list of IndexName Objects with same baseIndexName on which operations will be applied.
* @param purgeThresholdMillis after which a fully functional index is eligible for purge operations
*
* @return This method returns an IndexVersionOperation list i.e indexNameObjectList marked with operations
*/
public static List<IndexVersionOperation> generateIndexVersionOperationList(NodeState indexDefParentNode,
List<IndexName> indexNameObjectList, long purgeThresholdMillis) {
public static List<IndexVersionOperation> generateIndexVersionOperationList(NodeState rootNode, String parentPath,
List<IndexName> indexNameObjectList, long purgeThresholdMillis) {
NodeState indexDefParentNode = NodeStateUtils.getNode(rootNode, parentPath);
List<IndexName> reverseSortedIndexNameList = getReverseSortedIndexNameList(indexNameObjectList);
List<IndexVersionOperation> indexVersionOperationList = new LinkedList<>();

@@ -86,44 +92,71 @@ public static List<IndexVersionOperation> generateIndexVersionOperationList(Node
}

if (!reverseSortedIndexNameList.isEmpty()) {
IndexName activeIndexNameObject = reverseSortedIndexNameList.remove(0);
NodeState activeIndexNode = indexDefParentNode.getChildNode(PathUtils.getName(activeIndexNameObject.getNodeName()));
boolean isActiveIndexLongEnough = isIndexPurgeReady(activeIndexNameObject, activeIndexNode, purgeThresholdMillis);
int activeProductVersion = activeIndexNameObject.getProductVersion();
indexVersionOperationList.add(new IndexVersionOperation(activeIndexNameObject));

// for rest indexes except active index
for (IndexName indexNameObject : reverseSortedIndexNameList) {
String indexName = indexNameObject.getNodeName();
NodeState indexNode = indexDefParentNode.getChildNode(PathUtils.getName(indexName));
IndexVersionOperation indexVersionOperation = new IndexVersionOperation(indexNameObject);
// if active index not long enough, NOOP for all indexes
if (isActiveIndexLongEnough) {
if (indexNameObject.getProductVersion() == activeProductVersion && indexNameObject.getCustomerVersion() == 0) {
indexVersionOperation.setOperation(Operation.DELETE_HIDDEN_AND_DISABLE);
} else {
// the check hidden oak mount logic only works when passing through the proper composite store
if (isHiddenOakMountExists(indexNode)) {
LOG.info("Found hidden oak mount node for: '{}', disable it but no index definition deletion", indexName);
IndexName activeIndexNameObject = getActiveIndex(reverseSortedIndexNameList, parentPath, rootNode);
if (activeIndexNameObject == null) {
LOG.warn("Cannot find any active index from the list: {}", reverseSortedIndexNameList);
} else {
NodeState activeIndexNode = indexDefParentNode.getChildNode(PathUtils.getName(activeIndexNameObject.getNodeName()));
boolean isActiveIndexOldEnough = isActiveIndexOldEnough(activeIndexNameObject, activeIndexNode, purgeThresholdMillis);
int activeProductVersion = activeIndexNameObject.getProductVersion();
indexVersionOperationList.add(new IndexVersionOperation(activeIndexNameObject));

// the reverseSortedIndexNameList will now contain the remaining indexes,
// the active index and disabled index was removed from that list already
for (IndexName indexNameObject : reverseSortedIndexNameList) {
String indexName = indexNameObject.getNodeName();
NodeState indexNode = indexDefParentNode.getChildNode(PathUtils.getName(indexName));
IndexVersionOperation indexVersionOperation = new IndexVersionOperation(indexNameObject);
// if active index not long enough, NOOP for all indexes
if (isActiveIndexOldEnough) {
if (indexNameObject.getProductVersion() == activeProductVersion && indexNameObject.getCustomerVersion() == 0) {
indexVersionOperation.setOperation(Operation.DELETE_HIDDEN_AND_DISABLE);
} else if (indexNameObject.getProductVersion() <= activeProductVersion ) {
// the check hidden oak mount logic only works when passing through the proper composite store
if (isHiddenOakMountExists(indexNode)) {
LOG.info("Found hidden oak mount node for: '{}', disable it but no index definition deletion", indexName);
indexVersionOperation.setOperation(Operation.DELETE_HIDDEN_AND_DISABLE);
} else {
indexVersionOperation.setOperation(Operation.DELETE);
}
} else {
indexVersionOperation.setOperation(Operation.DELETE);
// if the index product version is larger than active index, leave it as is
// for instance: if there is active damAssetLucene-7, the inactive damAssetLucene-8 will be leave there as is
LOG.info("The index '{}' leave as is since the version is larger than current active index", indexName);
indexVersionOperation.setOperation(Operation.NOOP);
}
}
LOG.info("The operation for index '{}' will be: '{}'", indexName, indexVersionOperation.getOperation());
indexVersionOperationList.add(indexVersionOperation);
}
LOG.info("The operation for index '{}' will be: '{}'", indexName, indexVersionOperation.getOperation());
indexVersionOperationList.add(indexVersionOperation);
}
}
if (!isValidIndexVersionOperationList(indexVersionOperationList)) {
if (indexVersionOperationList.isEmpty() || !isValidIndexVersionOperationList(indexVersionOperationList)) {
LOG.info("Not valid version operation list: '{}', skip all", indexNameObjectList);
indexVersionOperationList = Collections.emptyList();
}
return indexVersionOperationList;
}

// iterate all indexes from high version to lower version to find the active index, then remove it from the reverseSortedIndexNameList
private static IndexName getActiveIndex(List<IndexName> reverseSortedIndexNameList, String parentPath, NodeState rootNode) {
for (int i = 0; i < reverseSortedIndexNameList.size(); i++) {
IndexName indexNameObject = reverseSortedIndexNameList.get(i);
String indexName = indexNameObject.getNodeName();
String indexPath = PathUtils.concat(parentPath, PathUtils.getName(indexName));
if (IndexName.isIndexActive(indexPath, rootNode)) {
LOG.info("Found active index '{}'", indexPath);
reverseSortedIndexNameList.remove(i);
return indexNameObject;
} else {
LOG.info("The index '{}' isn't active", indexPath);
}
}
return null;
}

// do index purge ready based on the active index's last reindexing time is longer enough, we do this for prevent rollback
private static boolean isIndexPurgeReady(IndexName activeIndexName, NodeState activeIndexNode, long purgeThresholdMillis) {
private static boolean isActiveIndexOldEnough(IndexName activeIndexName, NodeState activeIndexNode, long purgeThresholdMillis) {
// the 1st index must be active
String indexName = activeIndexName.getNodeName();
if (activeIndexNode.hasChildNode(IndexDefinition.STATUS_NODE)) {
@@ -135,16 +168,16 @@ private static boolean isIndexPurgeReady(IndexName activeIndexName, NodeState ac
long reindexCompletionTimeInMillis = PurgeOldVersionUtils.getMillisFromString(reindexCompletionTime);
long currentTimeInMillis = System.currentTimeMillis();
if (currentTimeInMillis - reindexCompletionTimeInMillis > purgeThresholdMillis) {
LOG.info("Found active index {} is longer enough", indexName);
LOG.info("Found active index {} is old enough", indexName);
return true;
} else {
LOG.info("The last index time '{}' isn't longer enough for: {}", reindexCompletionTime, indexName);
LOG.info("The last index time '{}' isn't old enough for: {}", reindexCompletionTime, indexName);
}
} else {
LOG.warn("{} property is not set for index {}", IndexDefinition.REINDEX_COMPLETION_TIMESTAMP, indexName);
}
}
LOG.info("The active index '{}' indexing time isn't long enough", indexName);
LOG.info("The active index '{}' indexing time isn't old enough", indexName);
return false;
}

@@ -38,7 +38,6 @@
import org.apache.jackrabbit.oak.spi.commit.EditorHook;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -76,18 +75,17 @@ public void execute(NodeStore nodeStore, boolean isReadWriteRepository, long pur

public List<IndexVersionOperation> getPurgeIndexes(NodeStore nodeStore, long purgeThresholdMillis, List<String> indexPaths) throws IOException, CommitFailedException {
List<IndexVersionOperation> purgeIndexList = new ArrayList<>();
LOG.info("Getting indexes to purge over node type '{}' and index paths '{}'", nodeStore.getClass().getSimpleName(), indexPaths);
LOG.info("Getting indexes to purge over index paths '{}'", indexPaths);
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(baseIndexPath);
List<IndexName> indexNameObjectList = getIndexNameObjectList(entry.getValue());
LOG.info("Validate purge index over base of '{}', which includes: '{}'", baseIndexPath,indexNameObjectList);
NodeState indexDefParentNode = NodeStateUtils.getNode(nodeStore.getRoot(), parentPath);
LOG.info("Validate purge index over base of '{}', which includes: '{}'", baseIndexPath, indexNameObjectList);
List<IndexVersionOperation> toDeleteIndexNameObjectList = IndexVersionOperation.generateIndexVersionOperationList(
indexDefParentNode, indexNameObjectList, purgeThresholdMillis);
nodeStore.getRoot(), parentPath, indexNameObjectList, purgeThresholdMillis);
toDeleteIndexNameObjectList.removeIf(item -> (item.getOperation() == IndexVersionOperation.Operation.NOOP));
if (!toDeleteIndexNameObjectList.isEmpty()) {
LOG.info("Found some index need to be purged over base'{}': '{}'", baseIndexPath, toDeleteIndexNameObjectList);
@@ -18,6 +18,10 @@
*/
package org.apache.jackrabbit.oak.indexversion;

import static org.apache.jackrabbit.commons.JcrUtils.getOrCreateByPath;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
@@ -51,9 +55,6 @@
import org.junit.Test;

import ch.qos.logback.classic.Level;
import static org.apache.jackrabbit.commons.JcrUtils.getOrCreateByPath;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;

public class PurgeOldIndexVersionTest extends AbstractIndexCommandTest {
private final static String FOO1_INDEX_PATH = "/oak:index/fooIndex1";
@@ -86,6 +87,8 @@ public void deleteOldIndexCompletely() throws Exception {
createCustomIndex(TEST_INDEX_PATH, 4, 1, false);
createCustomIndex(TEST_INDEX_PATH, 4, 2, false);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-2"));

runIndexPurgeCommand(true, 1, "");

NodeState indexRootNode = fixture.getNodeStore().getRoot().getChildNode("oak:index");
@@ -117,11 +120,44 @@ public void noDeleteIfActiveIndexTimeThresholdNotMeet() throws Exception {
createCustomIndex(TEST_INDEX_PATH, 4, 1, false);
createCustomIndex(TEST_INDEX_PATH, 4, 2, false);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-2"));

runIndexPurgeCommand(true, TimeUnit.DAYS.toMillis(1), "");

List<String> logs = custom.getLogs();
assertThat(logs.toString(),
containsString("The active index '/oak:index/fooIndex-4-custom-2' indexing time isn't long enough"));
containsString("The active index '/oak:index/fooIndex-4-custom-2' indexing time isn't old enough"));

NodeState indexRootNode = fixture.getNodeStore().getRoot().getChildNode("oak:index");
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-2-custom-1").exists());
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-3").exists());
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-3-custom-1").exists());
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-3-custom-2").exists());
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-4-custom-1").exists());
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-4-custom-2").exists());
} finally {
custom.finished();
}
}
@Test
public void noDeleteIfNoActiveIndex() throws Exception {
LogCustomizer custom = LogCustomizer.forLogger("org.apache.jackrabbit.oak.indexversion.IndexVersionOperation")
.enable(Level.INFO)
.create();
try {
custom.starting();
createTestData(false);
createCustomIndex(TEST_INDEX_PATH, 2, 1, false);
createCustomIndex(TEST_INDEX_PATH, 3, 0, false);
createCustomIndex(TEST_INDEX_PATH, 3, 1, false);
createCustomIndex(TEST_INDEX_PATH, 3, 2, false);
createCustomIndex(TEST_INDEX_PATH, 4, 1, false);
createCustomIndex(TEST_INDEX_PATH, 4, 2, false);

runIndexPurgeCommand(true, TimeUnit.DAYS.toMillis(1), "");

List<String> logs = custom.getLogs();
assertThat(logs.toString(), containsString("Cannot find any active index from the list: [/oak:index/fooIndex-4-custom-2"));

NodeState indexRootNode = fixture.getNodeStore().getRoot().getChildNode("oak:index");
Assert.assertTrue(indexRootNode.getChildNode("fooIndex-2-custom-1").exists());
@@ -160,6 +196,8 @@ public void noDeleteIfActiveIndexTimeMissing() throws Exception {
.removeProperty(IndexDefinition.REINDEX_COMPLETION_TIMESTAMP);
store.merge(rootBuilder, EmptyHook.INSTANCE, CommitInfo.EMPTY);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-2"));

runIndexPurgeCommand(true, 1, "");

List<String> logs = custom.getLogs();
@@ -194,6 +232,8 @@ public void noDeleteIfInvalidIndexOperationVersion() throws Exception {
createCustomIndex(TEST_INDEX_PATH, 4, 1, false);
createCustomIndex(TEST_INDEX_PATH, 4, 2, false);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-2"));

runIndexPurgeCommand(true, 1, "");

List<String> logs = custom.getLogs();
@@ -222,7 +262,7 @@ public void noDeleteForDisabledIndexesIfOakMountExists() throws Exception {
rootBuilder.getChildNode("oak:index").getChildNode("fooIndex-4-custom-1").setProperty("type", "disabled");
store.merge(rootBuilder, EmptyHook.INSTANCE, CommitInfo.EMPTY);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-1"));
addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-1", "fooIndex-4-custom-2"));

runIndexPurgeCommand(true, 1, "/oak:index/fooIndex,/oak:index");
NodeState indexRootNode = fixture.getNodeStore().getRoot().getChildNode("oak:index");
@@ -254,6 +294,8 @@ public void onlyDeleteVersionIndexesMentionedUnderIndexPaths() throws Exception
createCustomIndex(FOO1_INDEX_PATH, 3, 1, false);
createCustomIndex(FOO1_INDEX_PATH, 3, 2, false);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-2"));

runIndexPurgeCommand(true, 1, "/oak:index/fooIndex");

NodeState indexRootNode = fixture.getNodeStore().getRoot().getChildNode("oak:index");
@@ -288,6 +330,8 @@ public void noDeleteIfNonReadWriteMode() throws Exception {
createCustomIndex(TEST_INDEX_PATH, 4, 1, false);
createCustomIndex(TEST_INDEX_PATH, 4, 2, false);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-4-custom-2"));

runIndexPurgeCommand(false, 1, "");

List<String> logs = custom.getLogs();
@@ -309,7 +353,7 @@ public void disableIndexesIfOakMountExists() throws Exception {
createCustomIndex(TEST_INDEX_PATH, 4, 1, false);
createCustomIndex(TEST_INDEX_PATH, 4, 2, false);

addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-3", "fooIndex-3-custom-1"));
addMockHiddenOakMount(fixture.getNodeStore(), Arrays.asList("fooIndex-3", "fooIndex-3-custom-1", "fooIndex-4-custom-2"));

runIndexPurgeCommand(true, 1, "");

@@ -230,7 +230,7 @@ public String nextCustomizedName() {
return baseName + "-" + productVersion + "-custom-" + (customerVersion + 1);
}

private static boolean isIndexActive(String indexPath, NodeState rootState) {
public static boolean isIndexActive(String indexPath, NodeState rootState) {
// An index is active if it has a hidden child node that starts with ":oak:mount-",
// OR if it is an active merged index
try {
@@ -306,4 +306,4 @@ public boolean isLegal() {
return isLegal;
}

}
}

0 comments on commit 5ac5b17

Please sign in to comment.