Skip to content

Commit

Permalink
SOLR-17504: CoreContainer calls UpdateHandler.commit. (#2786)
Browse files Browse the repository at this point in the history
  • Loading branch information
bruno-roustant authored Nov 19, 2024
1 parent 72cc275 commit 5cc733b
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 19 deletions.
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ Improvements

* SOLR-17390: EmbeddedSolrServer now considers the ResponseParser (David Smiley)

* SOLR-17504: CoreContainer calls UpdateHandler.commit when closing a read-only core (Bruno Roustant)

Optimizations
---------------------
* SOLR-14985: Solrj CloudSolrClient with Solr URLs had serious performance regressions (since the
Expand Down
21 changes: 6 additions & 15 deletions solr/core/src/java/org/apache/solr/core/CoreContainer.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
import org.apache.http.client.CredentialsProvider;
import org.apache.http.config.Lookup;
import org.apache.lucene.index.CorruptIndexException;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.BytesRef;
Expand Down Expand Up @@ -95,6 +94,7 @@
import org.apache.solr.common.cloud.Replica.State;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.CollectionUtil;
import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.IOUtils;
Expand Down Expand Up @@ -138,6 +138,8 @@
import org.apache.solr.metrics.SolrMetricProducer;
import org.apache.solr.metrics.SolrMetricsContext;
import org.apache.solr.pkg.SolrPackageLoader;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.search.CacheConfig;
Expand All @@ -153,10 +155,10 @@
import org.apache.solr.security.PublicKeyHandler;
import org.apache.solr.security.SecurityPluginHolder;
import org.apache.solr.security.SolrNodeKeyPair;
import org.apache.solr.update.CommitUpdateCommand;
import org.apache.solr.update.SolrCoreState;
import org.apache.solr.update.UpdateShardHandler;
import org.apache.solr.util.OrderedExecutor;
import org.apache.solr.util.RefCounted;
import org.apache.solr.util.StartupLoggingUtils;
import org.apache.solr.util.stats.MetricUtils;
import org.apache.zookeeper.KeeperException;
Expand Down Expand Up @@ -2058,19 +2060,8 @@ public void reload(String name, UUID coreId) {

// force commit on old core if the new one is readOnly and prevent any new updates
if (newCore.readOnly) {
RefCounted<IndexWriter> iwRef = core.getSolrCoreState().getIndexWriter(null);
if (iwRef != null) {
IndexWriter iw = iwRef.get();
// switch old core to readOnly
core.readOnly = true;
try {
if (iw != null) {
iw.commit();
}
} finally {
iwRef.decref();
}
}
SolrQueryRequest req = new LocalSolrQueryRequest(core, new ModifiableSolrParams());
core.getUpdateHandler().commit(CommitUpdateCommand.closeOnCommit(req, false));
}

if (docCollection != null) {
Expand Down
32 changes: 32 additions & 0 deletions solr/core/src/java/org/apache/solr/update/CommitUpdateCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,21 @@

/** A commit index command encapsulated in an object. */
public class CommitUpdateCommand extends UpdateCommand {

// Behavior about opening a searcher after a hard commit.
// - Open a standard searcher and wait for it. (default)
// openSearcher == true && waitSearcher == true && closeSearcher == false
// - Open a standard searcher, but do not wait for it.
// openSearcher == true && waitSearcher == false && closeSearcher == false
// - Open a real-time searcher quicker without auto-warming.
// openSearcher == false && closeSearcher == false
// - Do not open any searcher and prevent further updates (e.g. when the core is closing).
// closeSearcher == true

public boolean optimize;
public boolean openSearcher = true; // open a new searcher as part of a hard commit
public boolean waitSearcher = true;
private boolean closeSearcher = false;
public boolean expungeDeletes = false;
public boolean softCommit = false;
public boolean prepareCommit = false;
Expand All @@ -47,6 +59,26 @@ public CommitUpdateCommand(SolrQueryRequest req, boolean optimize) {
this.optimize = optimize;
}

/**
* Creates a {@link CommitUpdateCommand} to commit before closing the core and also prevent any
* new update by setting the core in read-only mode. Does not open a new searcher.
*/
public static CommitUpdateCommand closeOnCommit(SolrQueryRequest req, boolean optimize) {
CommitUpdateCommand cmd = new CommitUpdateCommand(req, optimize);
cmd.openSearcher = false;
cmd.waitSearcher = false;
cmd.closeSearcher = true;
return cmd;
}

/**
* Indicates whether this command is a commit before the core is closed. Any new updates must be
* prevented after the commit.
*/
public boolean isClosingOnCommit() {
return closeSearcher;
}

@Override
public String name() {
return "commit";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,9 @@ public void commit(CommitUpdateCommand cmd) throws IOException {

RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
try {
if (cmd.isClosingOnCommit()) {
core.readOnly = true;
}
IndexWriter writer = iw.get();
if (cmd.optimize) {
writer.forceMerge(cmd.maxOptimizeSegments);
Expand Down Expand Up @@ -786,7 +789,7 @@ public void commit(CommitUpdateCommand cmd) throws IOException {
if (ulog != null) ulog.preSoftCommit(cmd);
if (cmd.openSearcher) {
core.getSearcher(true, false, waitSearcher);
} else {
} else if (!cmd.isClosingOnCommit()) {
// force open a new realtime searcher so realtime-get and versioning code can see the
// latest
RefCounted<SolrIndexSearcher> searchHolder = core.openNewSearcher(true, true);
Expand Down Expand Up @@ -971,8 +974,10 @@ public void closeWriter(IndexWriter writer) throws IOException {

// todo: refactor this shared code (or figure out why a real CommitUpdateCommand can't
// be used)
SolrIndexWriter.setCommitData(writer, cmd.getVersion(), null);
writer.commit();
if (shouldCommit(cmd, writer)) {
SolrIndexWriter.setCommitData(writer, cmd.getVersion(), cmd.commitData);
writer.commit();
}

synchronized (solrCoreState.getUpdateLock()) {
ulog.postCommit(cmd);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

<xi:include href="solrconfig.snippet.randomindexconfig.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>

<updateHandler class="solr.DirectUpdateHandler2">
<updateHandler class="${solr.updateHandler:solr.DirectUpdateHandler2}">

<autoCommit>
<maxTime>${solr.autoCommit.maxTime:-1}</maxTime>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* 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.solr.update;

import java.io.IOException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.lucene.index.IndexWriter;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.common.util.TimeSource;
import org.apache.solr.core.SolrCore;
import org.apache.solr.util.LogLevel;
import org.apache.solr.util.TimeOut;
import org.junit.BeforeClass;
import org.junit.Test;

/** Tests {@link DirectUpdateHandler2} with update log enabled. */
@LogLevel("org.apache.solr.update=INFO")
public class DirectUpdateHandlerWithUpdateLogTest extends SolrTestCaseJ4 {

@BeforeClass
public static void beforeClass() throws Exception {
System.setProperty("solr.updateHandler", SpyingUpdateHandler.class.getName());
initCore("solrconfig.xml", "schema11.xml");
}

@Test
public void testShouldCommitHook() throws Exception {
// Given a core.
SolrCore core = h.getCore();
assertNotNull(core);
SpyingUpdateHandler updater = (SpyingUpdateHandler) core.getUpdateHandler();
updater.shouldCommitCallCount.set(0);

// When we add a doc and commit.
assertU(adoc("id", "1"));
assertU(commit());
// Then the shouldCommit hook is called.
assertEquals(1, updater.shouldCommitCallCount.get());

// When we add a doc and soft commit.
assertU(adoc("id", "2"));
assertU(commit("softCommit", "true"));
// Then the shouldCommit hook is not called.
assertEquals(1, updater.shouldCommitCallCount.get());
// And when we commit.
assertU(commit());
// Then the shouldCommit hook is called.
assertEquals(2, updater.shouldCommitCallCount.get());

// When we commit with no updates (empty commit).
assertU(commit());
// Then the shouldCommit hook is called (may commit only user metadata).
assertEquals(3, updater.shouldCommitCallCount.get());

// When we add a doc, do not commit, and close the IndexWriter.
assertU(adoc("id", "3"));
h.close();
// Then the shouldCommit hook is called.
new TimeOut(10, TimeUnit.SECONDS, TimeSource.NANO_TIME)
.waitFor(
"Timeout waiting for should commit hook",
() -> updater.shouldCommitCallCount.get() == 4);
}

public static class SpyingUpdateHandler extends DirectUpdateHandler2 {

final AtomicInteger shouldCommitCallCount = new AtomicInteger();

public SpyingUpdateHandler(SolrCore core) {
super(core);
}

public SpyingUpdateHandler(SolrCore core, UpdateHandler updateHandler) {
super(core, updateHandler);
}

@Override
protected boolean shouldCommit(CommitUpdateCommand cmd, IndexWriter writer) throws IOException {
shouldCommitCallCount.incrementAndGet();
return super.shouldCommit(cmd, writer);
}
}
}

0 comments on commit 5cc733b

Please sign in to comment.