You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by am...@apache.org on 2021/03/09 11:41:24 UTC
svn commit: r1887362 - in /jackrabbit/oak/trunk/oak-search-elastic/src:
main/java/org/apache/jackrabbit/oak/plugins/index/elastic/
test/java/org/apache/jackrabbit/oak/plugins/index/elastic/
Author: amrverma
Date: Tue Mar 9 11:41:23 2021
New Revision: 1887362
URL: http://svn.apache.org/viewvc?rev=1887362&view=rev
Log:
OAK-9375: Remote elastic index deletion job incorrectly deletes indices
Modified:
jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleaner.java
jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexNameHelper.java
jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java
jackrabbit/oak/trunk/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleanerTest.java
Modified: jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleaner.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleaner.java?rev=1887362&r1=1887361&r2=1887362&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleaner.java (original)
+++ jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleaner.java Tue Mar 9 11:41:23 2021
@@ -40,6 +40,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
@@ -89,6 +90,7 @@ public class ElasticIndexCleaner impleme
filter(index -> Arrays.stream(remoteIndices).noneMatch(remoteIndex -> remoteIndex.equals(index))).collect(Collectors.toList());
externallyDeletedIndices.forEach(danglingRemoteIndicesMap::remove);
Set<String> existingIndices = new HashSet<>();
+ AtomicBoolean shouldReturnSilently = new AtomicBoolean(false);
root.getChildNode(INDEX_DEFINITIONS_NAME).getChildNodeEntries().forEach(childNodeEntry -> {
PropertyState typeProperty = childNodeEntry.getNodeState().getProperty(IndexConstants.TYPE_PROPERTY_NAME);
String typeValue = typeProperty != null ? typeProperty.getValue(Type.STRING) : "";
@@ -96,15 +98,31 @@ public class ElasticIndexCleaner impleme
If index type is "elasticsearch" or "disabled", we try to find remote index name. In case of disabled lucene or
property indices, the remote index name would be null. So only elasticsearch indices are affected here.
*/
- if (typeValue.equals(ElasticIndexDefinition.TYPE_ELASTICSEARCH) || typeValue.equals("disabled")) {
+ if (ElasticIndexDefinition.TYPE_ELASTICSEARCH.equals(typeValue) || "disabled".equals(typeValue)) {
String indexPath = "/" + INDEX_DEFINITIONS_NAME + "/" + childNodeEntry.getName();
String remoteIndexName = ElasticIndexNameHelper.getRemoteIndexName(indexPrefix, childNodeEntry.getNodeState(), indexPath);
if (remoteIndexName != null) {
existingIndices.add(remoteIndexName);
+ } else if (ElasticIndexDefinition.TYPE_ELASTICSEARCH.equals(typeValue)){
+ /*
+ Didn't check for disabled indexes in this "else if" condition because an index could be disabled at three stages
+ and the following should hold -
+ Stage 1 - index definition was created with type="disabled". This means indexing has not been done for index and
+ hence remote index name and the remote index itself shouldn't exist
+ Stage 2 - index was disabled after indexing was complete - in this case we would find remote index name and won't
+ enter this condition
+ Stage 3 - index was disabled after indexing started but before it was complete. Ideally this should only be done in
+ case of some error in indexing and after interrupting and pausing indexing lane. So in this case it should be safe to
+ delete the partially created remote index as it should be recreated by reindexing.
+ */
+ LOG.info("Could not obtain remote index name for {}. Won't delete any index.", childNodeEntry.getName());
+ shouldReturnSilently.set(true);
}
}
});
-
+ if (shouldReturnSilently.get()) {
+ return;
+ }
List<String> indicesToDelete = new ArrayList<>();
for (String remoteIndexName : remoteIndices) {
if (!existingIndices.contains(remoteIndexName)) {
Modified: jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexNameHelper.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexNameHelper.java?rev=1887362&r1=1887361&r2=1887362&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexNameHelper.java (original)
+++ jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexNameHelper.java Tue Mar 9 11:41:23 2021
@@ -23,6 +23,8 @@ import org.apache.jackrabbit.oak.commons
import org.apache.jackrabbit.oak.plugins.index.IndexConstants;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.UUID;
import java.util.regex.Pattern;
@@ -33,6 +35,8 @@ import static org.elasticsearch.common.S
public class ElasticIndexNameHelper {
+ private static final Logger LOG = LoggerFactory.getLogger(ElasticIndexNameHelper.class);
+
private static final int MAX_NAME_LENGTH = 255;
private static final String INVALID_CHARS_REGEX = Pattern.quote(INVALID_FILENAME_CHARS
@@ -57,6 +61,7 @@ public class ElasticIndexNameHelper {
}
PropertyState seedProp = indexNode.getProperty(ElasticIndexDefinition.PROP_INDEX_NAME_SEED);
if (seedProp == null) {
+ LOG.debug("Could not obtain remote index name. No seed found for index {}", indexPath);
return null;
}
long seed = seedProp.getValue(Type.LONG);
Modified: jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java?rev=1887362&r1=1887361&r2=1887362&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java (original)
+++ jackrabbit/oak/trunk/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java Tue Mar 9 11:41:23 2021
@@ -110,11 +110,13 @@ public class ElasticIndexProviderService
description = "Local file system path where text extraction cache stores/load entries to recover from timed out operation")
String localTextExtractionDir();
- @AttributeDefinition(name = "Remote index cleanup frequency", description = "Frequency (in seconds) of running remote index deletion scheduled task")
- int remoteIndexCleanupFrequency() default 60;
-
- @AttributeDefinition(name = "Remote index deletion threshold", description = "Time in seconds after which a remote index whose local index is not found gets deleted")
- int remoteIndexDeletionThreshold() default 300;
+ @AttributeDefinition(name = "Remote index cleanup frequency", description = "Frequency (in seconds) of running remote index deletion scheduled task." +
+ "Set this to -1 to disable the task. Default is 1 hour.")
+ int remoteIndexCleanupFrequency() default 60*60;
+
+ @AttributeDefinition(name = "Remote index deletion threshold", description = "Time in seconds after which a remote index whose local index is not found gets deleted." +
+ "Default is 1 day.")
+ int remoteIndexDeletionThreshold() default 24*60*60;
}
@@ -184,6 +186,9 @@ public class ElasticIndexProviderService
if (!reachable) {
throw new IllegalArgumentException("Elastic server is not available - " + elasticConnection.toString());
}
+ if (contextConfig.remoteIndexCleanupFrequency() == -1) {
+ return;
+ }
ElasticIndexCleaner task = new ElasticIndexCleaner(elasticConnection, nodeStore, contextConfig.remoteIndexDeletionThreshold());
oakRegs.add(scheduleWithFixedDelay(whiteboard, task, contextConfig.remoteIndexCleanupFrequency()));
}
Modified: jackrabbit/oak/trunk/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleanerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleanerTest.java?rev=1887362&r1=1887361&r2=1887362&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleanerTest.java (original)
+++ jackrabbit/oak/trunk/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexCleanerTest.java Tue Mar 9 11:41:23 2021
@@ -139,4 +139,31 @@ public class ElasticIndexCleanerTest ext
}
+ @Test
+ public void preventIndexDeletionWhenSeedNotFound() throws Exception {
+ int indexDeletionThresholdTime = 5;
+ String indexId = createIndexAndContentNode("propa", "test1");
+ String indexPath = "/" + INDEX_DEFINITIONS_NAME + "/" + indexId;
+ NodeState oakIndex = nodeStore.getRoot().getChildNode(INDEX_DEFINITIONS_NAME);
+ NodeState indexState = oakIndex.getChildNode(indexId);
+ indexState.builder().remove();
+
+ root.refresh();
+ root.getTree(indexPath).removeProperty(ElasticIndexDefinition.PROP_INDEX_NAME_SEED);
+ root.commit();
+
+ ElasticIndexCleaner cleaner = new ElasticIndexCleaner(esConnection, nodeStore, indexDeletionThresholdTime);
+ cleaner.run();
+
+ String remoteIndexName = ElasticIndexNameHelper.getRemoteIndexName(esConnection.getIndexPrefix(), indexState,
+ indexPath);
+ assertTrue(esConnection.getClient().indices().exists(new GetIndexRequest(remoteIndexName), RequestOptions.DEFAULT));
+
+ Thread.sleep(TimeUnit.SECONDS.toMillis(indexDeletionThresholdTime));
+ cleaner.run();
+
+ assertTrue(esConnection.getClient().indices().exists(new GetIndexRequest(remoteIndexName), RequestOptions.DEFAULT));
+
+ }
+
}