diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexerMBeanImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexerMBeanImpl.java index 39f7bcf8e45..026cc5605fe 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexerMBeanImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexerMBeanImpl.java @@ -33,6 +33,7 @@ import org.apache.jackrabbit.oak.plugins.index.importer.IndexImporterProvider; import org.apache.jackrabbit.oak.spi.state.Clusterable; import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.apache.jackrabbit.oak.spi.toggle.Feature; import org.apache.jackrabbit.oak.spi.whiteboard.Registration; import org.apache.jackrabbit.oak.spi.whiteboard.Tracker; import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; @@ -49,6 +50,14 @@ @Component(service = {}) public class IndexerMBeanImpl extends AnnotatedStandardMBean implements IndexerMBean { + + /** + * Feature toggle for reverting to the legacy out-of-band import flow. Disabled by default, + * so the new generalized async-alignment is used; enable it (via LaunchDarkly) to fall back + * to the previous elasticsearch-only handling. The LaunchDarkly flag key is "OAK-12307". + */ + static final String LEGACY_INDEX_IMPORT_TOGGLE = "OAK-12307"; + private final Logger log = LoggerFactory.getLogger(getClass()); @Reference private NodeStore nodeStore; @@ -59,6 +68,7 @@ public class IndexerMBeanImpl extends AnnotatedStandardMBean implements IndexerM private WhiteboardIndexEditorProvider editorProvider = new WhiteboardIndexEditorProvider(); private Registration mbeanReg; private Tracker providerTracker; + private Feature legacyImportFeature; public IndexerMBeanImpl() { super(IndexerMBean.class); @@ -73,7 +83,8 @@ public boolean importIndex(String indexDirPath) throws IOException, CommitFailed public boolean importIndex(String indexDirPath, boolean ignoreLocalLock) throws IOException, CommitFailedException { try { - IndexImporter importer = new IndexImporter(nodeStore, new File(indexDirPath), editorProvider, createLock(ignoreLocalLock)); + boolean useLegacyImportFlow = legacyImportFeature != null && legacyImportFeature.isEnabled(); + IndexImporter importer = new IndexImporter(nodeStore, new File(indexDirPath), editorProvider, createLock(ignoreLocalLock), useLegacyImportFlow); providerTracker.getServices().forEach(importer::addImporterProvider); importer.importIndex(); } catch (IOException | CommitFailedException | RuntimeException e) { @@ -106,6 +117,7 @@ private void activate(BundleContext context) { IndexerMBean.TYPE, "Indexer operations related MBean"); providerTracker = wb.track(IndexImporterProvider.class); + legacyImportFeature = Feature.newFeature(LEGACY_INDEX_IMPORT_TOGGLE, wb); } @Deactivate @@ -117,6 +129,9 @@ private void deactivate() { if (providerTracker != null) { providerTracker.stop(); } + if (legacyImportFeature != null) { + legacyImportFeature.close(); + } } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporter.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporter.java index eba58ec5124..2bc1b23848e 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporter.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporter.java @@ -98,21 +98,34 @@ public class IndexImporter { private final Set indexPathsToUpdate; private final StatisticsProvider statisticsProvider; private final IndexingReporter indexingReporter; + private final boolean useLegacyImportFlow; public IndexImporter(NodeStore nodeStore, File indexDir, IndexEditorProvider indexEditorProvider, AsyncIndexerLock indexerLock) throws IOException { - this(nodeStore, indexDir, indexEditorProvider, indexerLock, StatisticsProvider.NOOP, IndexingReporter.NOOP); + this(nodeStore, indexDir, indexEditorProvider, indexerLock, false); + } + + public IndexImporter(NodeStore nodeStore, File indexDir, IndexEditorProvider indexEditorProvider, + AsyncIndexerLock indexerLock, boolean useLegacyImportFlow) throws IOException { + this(nodeStore, indexDir, indexEditorProvider, indexerLock, StatisticsProvider.NOOP, IndexingReporter.NOOP, useLegacyImportFlow); } public IndexImporter(NodeStore nodeStore, File indexDir, IndexEditorProvider indexEditorProvider, AsyncIndexerLock indexerLock, StatisticsProvider statisticsProvider) throws IOException { - this(nodeStore, indexDir, indexEditorProvider, indexerLock, statisticsProvider, IndexingReporter.NOOP); + this(nodeStore, indexDir, indexEditorProvider, indexerLock, statisticsProvider, IndexingReporter.NOOP, false); } public IndexImporter(NodeStore nodeStore, File indexDir, IndexEditorProvider indexEditorProvider, AsyncIndexerLock indexerLock, StatisticsProvider statisticsProvider, IndexingReporter indexingReporter) throws IOException { + this(nodeStore, indexDir, indexEditorProvider, indexerLock, statisticsProvider, indexingReporter, false); + } + + public IndexImporter(NodeStore nodeStore, File indexDir, IndexEditorProvider indexEditorProvider, + AsyncIndexerLock indexerLock, StatisticsProvider statisticsProvider, IndexingReporter indexingReporter, + boolean useLegacyImportFlow) throws IOException { this.statisticsProvider = statisticsProvider; this.indexingReporter = indexingReporter; + this.useLegacyImportFlow = useLegacyImportFlow; checkArgument(indexDir.exists() && indexDir.isDirectory(), "Path [%s] does not point to existing directory", indexDir.getAbsolutePath()); this.nodeStore = nodeStore; @@ -209,17 +222,37 @@ void switchLanes() throws CommitFailedException { indexPathsToUpdate.add(indexInfo.indexPath); String idxBuilderType = idxBuilder.getString(TYPE_PROPERTY_NAME); - // check if provided index definitions is of different type than existing one - // also check if one of them is an elasticsearch type - if (idxBuilderType != null && - !idxBuilderType.equals(indexInfo.type) && - (idxBuilderType.equals(TYPE_ELASTICSEARCH) || indexInfo.type.equals(TYPE_ELASTICSEARCH))) { - - LOG.info("Provided index [{}] has a different type compared to the existing index." + - " Using lane from the index definition provided", indexInfo.indexPath); - - PropertyState asyncProperty = PropertyStates.createProperty(ASYNC_PROPERTY_NAME, List.of(indexInfo.asyncLaneName), Type.STRINGS); - idxBuilder.setProperty(asyncProperty); + if (useLegacyImportFlow) { + // Legacy behaviour: only realign async when the type mismatch involves an + // elasticsearch index, using the lane from the provided definition. + if (idxBuilderType != null && + !idxBuilderType.equals(indexInfo.type) && + (idxBuilderType.equals(TYPE_ELASTICSEARCH) || indexInfo.type.equals(TYPE_ELASTICSEARCH))) { + + LOG.info("Provided index [{}] has a different type compared to the existing index." + + " Using lane from the index definition provided", indexInfo.indexPath); + + PropertyState asyncProperty = PropertyStates.createProperty(ASYNC_PROPERTY_NAME, List.of(indexInfo.asyncLaneName), Type.STRINGS); + idxBuilder.setProperty(asyncProperty); + } + } else { + // If the provided index definition has a different type than the one on disk + // (e.g. disabled -> lucene, lucene -> elasticsearch), the on-disk definition is + // obsolete: the rebuilt index must reflect only the provided definition. Align the + // async property to the provided definition before switching lanes, so its lane + // (or its absence, for a sync index) is what gets restored afterwards. + if (idxBuilderType != null && !idxBuilderType.equals(indexInfo.type)) { + LOG.info("Existing index [{}] has a different type than the provided definition ([{}] -> [{}]);" + + " using the provided definition", indexInfo.indexPath, idxBuilderType, indexInfo.type); + + PropertyState providedAsync = indexDefinitionUpdater.getIndexState(indexInfo.indexPath) + .getProperty(ASYNC_PROPERTY_NAME); + if (providedAsync != null) { + idxBuilder.setProperty(PropertyStates.createProperty(ASYNC_PROPERTY_NAME, providedAsync.getValue(Type.STRINGS), Type.STRINGS)); + } else { + idxBuilder.removeProperty(ASYNC_PROPERTY_NAME); + } + } } AsyncLaneSwitcher.switchLane(idxBuilder, AsyncLaneSwitcher.getTempLaneName(indexInfo.asyncLaneName)); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/package-info.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/package-info.java index 0338da120ef..ff488f515ce 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/package-info.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/importer/package-info.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("0.5.0") +@Version("0.6.0") package org.apache.jackrabbit.oak.plugins.index.importer; import org.osgi.annotation.versioning.Version; \ No newline at end of file diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporterTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporterTest.java index 260967d5c01..3b2da23c9ea 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporterTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/importer/IndexImporterTest.java @@ -560,6 +560,74 @@ public void unlock(LockToken token) throws CommitFailedException { } } + @Test + public void importExistingDisabledIndexMissingAsyncKeepsAsync() throws Exception { + // Reindex output (index-definitions.json + index dir) carries async=async + String checkpoint = importDataIncrementalUpdateBeforeSetupMethod(); + + // The on-disk index is a disabled definition that is missing the async property + NodeBuilder builder = store.getRoot().builder(); + NodeBuilder idx = builder.child(INDEX_DEFINITIONS_NAME).child("fooIndex"); + idx.setProperty(TYPE_PROPERTY_NAME, TYPE_DISABLED); + idx.removeProperty(ASYNC_PROPERTY_NAME); + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + + IndexImporter importer = new IndexImporter(store, temporaryFolder.getRoot(), provider, NOOP_LOCK); + importer.addImporterProvider(getImporterProvider(checkpoint)); + importer.importIndex(); + + NodeState result = store.getRoot().getChildNode("oak:index").getChildNode("fooIndex"); + assertEquals("async", IndexImporter.getAsyncLaneName("/oak:index/fooIndex", result)); + } + + @Test + public void switchLanesTypeChangeToSyncDropsAsync() throws Exception { + NodeBuilder builder = store.getRoot().builder(); + // incoming (dumped) definition is a sync property index without async + builder.child("idx-a").setProperty("type", "property"); + + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + createIndexDirs("/idx-a"); + + // on-disk the index is an async lucene index + builder.child("idx-a").setProperty("type", "lucene"); + builder.child("idx-a").setProperty("async", "async"); + + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + + IndexImporter importer = new IndexImporter(store, temporaryFolder.getRoot(), provider, NOOP_LOCK); + importer.switchLanes(); + + // incoming definition is sync, so the on-disk async must not be carried over as previous + NodeState idxa = NodeStateUtils.getNode(store.getRoot(), "/idx-a"); + assertEquals(AsyncLaneSwitcher.ASYNC_PREVIOUS_NONE, idxa.getString(ASYNC_PREVIOUS)); + } + + @Test + public void switchLanesTypeChangeToSyncKeepsAsyncInLegacyFlow() throws Exception { + NodeBuilder builder = store.getRoot().builder(); + // incoming (dumped) definition is a sync property index without async + builder.child("idx-a").setProperty("type", "property"); + + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + createIndexDirs("/idx-a"); + + // on-disk the index is an async lucene index + builder.child("idx-a").setProperty("type", "lucene"); + builder.child("idx-a").setProperty("async", "async"); + + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + + // opt into the legacy flow (last arg true) to verify the pre-OAK-12307, + // elasticsearch-only behavior: a non-elasticsearch type change leaves the + // on-disk async in place. The default (no flag) uses the fix instead. + IndexImporter importer = new IndexImporter(store, temporaryFolder.getRoot(), provider, NOOP_LOCK, true); + importer.switchLanes(); + + NodeState idxa = NodeStateUtils.getNode(store.getRoot(), "/idx-a"); + assertEquals("async", idxa.getString(ASYNC_PREVIOUS)); + } + private static FilterImpl createFilter(NodeState root, String nodeTypeName) { NodeTypeInfoProvider nodeTypes = new NodeStateNodeTypeInfoProvider(root); NodeTypeInfo type = nodeTypes.getNodeTypeInfo(nodeTypeName); diff --git a/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/IndexImporterSupportBase.java b/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/IndexImporterSupportBase.java index 19aa30c76b3..76226d00da1 100644 --- a/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/IndexImporterSupportBase.java +++ b/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/IndexImporterSupportBase.java @@ -41,13 +41,16 @@ public IndexImporterSupportBase(IndexHelper indexHelper) { public void importIndex(File importDir) throws IOException, CommitFailedException { try (IndexEditorProvider providers = createIndexEditorProvider()) { + // Offline import has no LaunchDarkly to read the legacy toggle from; oak-run always + // uses the new import flow (never the legacy one). IndexImporter importer = new IndexImporter( nodeStore, importDir, providers, createLock(), indexHelper.getStatisticsProvider(), - indexHelper.getIndexReporter()); + indexHelper.getIndexReporter(), + false); addImportProviders(importer); importer.importIndex(); }