fabriziofortino commented on code in PR #3085:
URL: https://github.com/apache/jackrabbit-oak/pull/3085#discussion_r3881082963
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexEditorContext.java:
##########
@@ -50,8 +51,8 @@ public DocumentMaker<ElasticDocument>
newDocumentMaker(IndexDefinition.IndexingR
}
@Override
- public ElasticIndexWriter getWriter() {
- return (ElasticIndexWriter) super.getWriter();
+ public FulltextIndexWriter<ElasticDocument> getWriter() {
+ return super.getWriter();
Review Comment:
there is actually no need for that. We could remove it
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexWriter.java:
##########
@@ -62,46 +62,46 @@ class ElasticIndexWriter implements
FulltextIndexWriter<ElasticDocument> {
private final ElasticConnection elasticConnection;
private final ElasticIndexDefinition indexDefinition;
private final ElasticBulkProcessorHandler bulkProcessorHandler;
- private final boolean reindex;
+ private final boolean requiresProvisioning;
private final String indexName;
private final ElasticRetryPolicy retryPolicy;
+ private final NodeBuilder definitionBuilder;
Review Comment:
only needed in the constructor. Can we remove it?
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/LazyElasticIndexWriter.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.index.elastic.index;
+
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticConnection;
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticIndexDefinition;
+import
org.apache.jackrabbit.oak.plugins.index.search.spi.editor.FulltextIndexWriter;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.function.Supplier;
+
+/**
+ * A {@link FulltextIndexWriter} proxy that defers creation of the real {@link
ElasticIndexWriter}
+ * to the first {@link #updateDocument}, {@link #deleteDocumentTree} or {@link
#deleteDocument}
+ * call (OAK-12249).
+ *
+ * <p>If {@link #close} is called before any document is written — i.e. the
reindex produced zero
+ * documents — the supplier is never invoked, so no Elasticsearch index or
alias is created.
+ * Instead, {@link ElasticIndexDefinition#PROP_REQUIRES_PROVISIONING} is set
on the definition
+ * node so the next incremental-write cycle provisions the index on demand. If
the index was
+ * already provisioned before this reindex (i.e. it previously had documents),
its now-stale
+ * alias and backing index are removed — otherwise pre-reindex content would
keep being served
+ * indefinitely.
+ *
+ * <p>The supplier is expected to create and provision the index as a side
effect of construction
+ * (as {@link ElasticIndexWriter} does). Thread safety is not required: Oak
calls each writer
+ * instance from a single thread.
+ */
+class LazyElasticIndexWriter implements FulltextIndexWriter<ElasticDocument> {
+ private static final Logger LOG =
LoggerFactory.getLogger(LazyElasticIndexWriter.class);
+
+ private final Supplier<ElasticIndexWriter> writerSupplier;
+ private final NodeBuilder definitionBuilder;
+ private final ElasticConnection elasticConnection;
+ private final ElasticIndexDefinition indexDefinition;
+ private ElasticIndexWriter delegate;
+
+ LazyElasticIndexWriter(Supplier<ElasticIndexWriter> writerSupplier,
NodeBuilder definitionBuilder,
+ ElasticConnection elasticConnection,
ElasticIndexDefinition indexDefinition) {
+ this.writerSupplier = writerSupplier;
+ this.definitionBuilder = definitionBuilder;
+ this.elasticConnection = elasticConnection;
+ this.indexDefinition = indexDefinition;
+ }
+
+ @Override
+ public void updateDocument(String path, ElasticDocument doc) throws
IOException {
+ getOrCreate().updateDocument(path, doc);
+ }
+
+ @Override
+ public void deleteDocumentTree(String path) throws IOException {
+ getOrCreate().deleteDocumentTree(path);
+ }
+
+ @Override
+ public void deleteDocument(String path) throws IOException {
+ getOrCreate().deleteDocument(path);
+ }
+
+ @Override
+ public boolean close(long timestamp) throws IOException {
+ if (delegate == null) {
+ LOG.info("Reindex produced no documents — skipping ES index
creation (OAK-12249)");
+
definitionBuilder.setProperty(ElasticIndexDefinition.PROP_REQUIRES_PROVISIONING,
true);
+ ElasticIndexWriter.unaliasIfProvisioned(elasticConnection,
indexDefinition);
Review Comment:
what about moving `unaliasIfProvisioned` in this class (no need to be
static)?
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/package-info.java:
##########
@@ -0,0 +1,20 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+@Version("2.4.1")
Review Comment:
see https://github.com/apache/jackrabbit-oak/pull/3088/changes
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/LazyProvisioningReindexITTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.index.elastic.index;
+
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticConnection;
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticConnectionRule;
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticIndexDefinition;
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticIndexStatistics;
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticIndexTracker;
+import org.apache.jackrabbit.oak.plugins.index.elastic.ElasticMetricHandler;
+import
org.apache.jackrabbit.oak.plugins.index.elastic.util.ElasticIndexDefinitionBuilder;
+import
org.apache.jackrabbit.oak.plugins.index.search.spi.editor.FulltextIndexWriter;
+import
org.apache.jackrabbit.oak.plugins.index.search.util.IndexDefinitionBuilder;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.stats.StatisticsProvider;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.InitialContentHelper.INITIAL_CONTENT;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * OAK-12249: a reindex under lazy provisioning that produces zero documents
must not leave a
+ * previously-provisioned index's stale alias and backing index behind —
otherwise the system
+ * keeps serving pre-reindex content indefinitely, with no signal that
anything is wrong.
+ */
+public class LazyProvisioningReindexITTest {
+
+ @ClassRule
+ public static final ElasticConnectionRule elasticRule = new
ElasticConnectionRule();
+
+ private ElasticConnection connection;
+ private ElasticIndexTracker indexTracker;
+ private NodeStore nodeStore;
+
+ @Before
+ public void setup() {
+ this.connection = elasticRule.useDocker() ?
+ elasticRule.getElasticConnectionForDocker() :
+ elasticRule.getElasticConnectionFromString();
+ this.indexTracker = new ElasticIndexTracker(connection, new
ElasticMetricHandler(StatisticsProvider.NOOP));
+ this.nodeStore = new MemoryNodeStore(INITIAL_CONTENT);
+ }
Review Comment:
you should extend `ElasticAbstractQueryTest` and remove this whole block
--
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]
