From 82d641cb88f57ca72fe26047721e9b391027cc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Wed, 10 Jan 2024 17:35:30 +0100 Subject: [PATCH 1/9] Migrate HttpSolrClient to Http2SolrClient with refactoring in order to use one service to query solr, and another service to manage index --- .../edu/harvard/iq/dataverse/DatasetPage.java | 4 +- .../search/AbstractSolrClientService.java | 46 +++++++++ .../iq/dataverse/search/IndexServiceBean.java | 93 ++++++++----------- .../search/SolrClientIndexService.java | 46 +++++++++ .../dataverse/search/SolrClientService.java | 36 +------ .../search/SolrIndexServiceBean.java | 4 +- .../search/IndexServiceBeanTest.java | 59 +++++------- .../search/SolrClientIndexServiceTest.java | 42 +++++++++ .../search/SolrClientServiceTest.java | 6 +- 9 files changed, 208 insertions(+), 128 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java create mode 100644 src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index b79f387f20b..b720b6a1b5c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -154,7 +154,7 @@ import edu.harvard.iq.dataverse.util.FileMetadataUtil; import java.util.Comparator; import org.apache.solr.client.solrj.SolrQuery; -import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.impl.BaseHttpSolrClient.RemoteSolrException; import org.apache.solr.client.solrj.response.FacetField; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrDocument; @@ -991,7 +991,7 @@ public Set getFileIdsInVersionFromSolr(Long datasetVersionId, String patte try { queryResponse = solrClientService.getSolrClient().query(solrQuery); - } catch (HttpSolrClient.RemoteSolrException ex) { + } catch (RemoteSolrException ex) { logger.fine("Remote Solr Exception: " + ex.getLocalizedMessage()); String msg = ex.getLocalizedMessage(); if (msg.contains(SearchFields.FILE_DELETED)) { diff --git a/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java b/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java new file mode 100644 index 00000000000..f36c4a9e591 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java @@ -0,0 +1,46 @@ +package edu.harvard.iq.dataverse.search; + +import java.io.IOException; +import java.util.logging.Logger; + +import org.apache.solr.client.solrj.SolrClient; + +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.util.SystemConfig; +import jakarta.ejb.EJB; + +public abstract class AbstractSolrClientService { + private static final Logger logger = Logger.getLogger(AbstractSolrClientService.class.getCanonicalName()); + + @EJB + SystemConfig systemConfig; + + public abstract void init(); + public abstract void close(); + public abstract SolrClient getSolrClient(); + public abstract void setSolrClient(SolrClient solrClient); + + public void close(SolrClient solrClient) { + if (solrClient != null) { + try { + solrClient.close(); + } catch (IOException e) { + logger.warning("Solr closing error: " + e); + } + solrClient = null; + } + } + + public void reInitialize() { + close(); + init(); + } + + public String getSolrUrl() { + // Get from MPCONFIG. Might be configured by a sysadmin or simply return the + // default shipped with resources/META-INF/microprofile-config.properties. + final String protocol = JvmSettings.SOLR_PROT.lookup(); + final String path = JvmSettings.SOLR_PATH.lookup(); + return protocol + "://" + this.systemConfig.getSolrHostColonPort() + path; + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java index 9e73c38a5d0..189cfb5de6c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java @@ -1,6 +1,28 @@ package edu.harvard.iq.dataverse.search; -import edu.harvard.iq.dataverse.*; +import edu.harvard.iq.dataverse.ControlledVocabularyValue; +import edu.harvard.iq.dataverse.DataFile; +import edu.harvard.iq.dataverse.DataFileServiceBean; +import edu.harvard.iq.dataverse.DataFileTag; +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetField; +import edu.harvard.iq.dataverse.DatasetFieldCompoundValue; +import edu.harvard.iq.dataverse.DatasetFieldConstant; +import edu.harvard.iq.dataverse.DatasetFieldServiceBean; +import edu.harvard.iq.dataverse.DatasetFieldType; +import edu.harvard.iq.dataverse.DatasetFieldValueValidator; +import edu.harvard.iq.dataverse.DatasetLinkingServiceBean; +import edu.harvard.iq.dataverse.DatasetServiceBean; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.Dataverse; +import edu.harvard.iq.dataverse.DataverseLinkingServiceBean; +import edu.harvard.iq.dataverse.DataverseServiceBean; +import edu.harvard.iq.dataverse.DvObject; +import edu.harvard.iq.dataverse.DvObjectServiceBean; +import edu.harvard.iq.dataverse.Embargo; +import edu.harvard.iq.dataverse.FileMetadata; +import edu.harvard.iq.dataverse.GlobalId; +import edu.harvard.iq.dataverse.PermissionServiceBean; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinUserServiceBean; import edu.harvard.iq.dataverse.batch.util.LoggingUtil; @@ -14,7 +36,6 @@ import edu.harvard.iq.dataverse.harvest.client.HarvestingClient; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; -import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.FileUtil; import edu.harvard.iq.dataverse.util.StringUtil; import edu.harvard.iq.dataverse.util.SystemConfig; @@ -39,8 +60,6 @@ import java.util.function.Function; import java.util.logging.Logger; import java.util.stream.Collectors; -import jakarta.annotation.PostConstruct; -import jakarta.annotation.PreDestroy; import jakarta.ejb.AsyncResult; import jakarta.ejb.Asynchronous; import jakarta.ejb.EJB; @@ -55,11 +74,9 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrQuery.SortClause; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.response.UpdateResponse; import org.apache.solr.common.SolrDocument; @@ -109,16 +126,15 @@ public class IndexServiceBean { @EJB SettingsServiceBean settingsService; @EJB - SolrClientService solrClientService; + SolrClientService solrClientService; // only for query index on Solr + @EJB + SolrClientIndexService solrClientIndexService; // only for add, update, or remove index on Solr @EJB DataFileServiceBean dataFileService; @EJB VariableServiceBean variableService; - - @EJB - IndexBatchServiceBean indexBatchService; - + @EJB DatasetFieldServiceBean datasetFieldService; @@ -138,37 +154,10 @@ public class IndexServiceBean { private static final String IN_REVIEW_STRING = "In Review"; private static final String DEACCESSIONED_STRING = "Deaccessioned"; public static final String HARVESTED = "Harvested"; - private String rootDataverseName; private Dataverse rootDataverseCached; - SolrClient solrServer; private VariableMetadataUtil variableMetadataUtil; - @PostConstruct - public void init() { - // Get from MPCONFIG. Might be configured by a sysadmin or simply return the default shipped with - // resources/META-INF/microprofile-config.properties. - String protocol = JvmSettings.SOLR_PROT.lookup(); - String path = JvmSettings.SOLR_PATH.lookup(); - - String urlString = protocol + "://" + systemConfig.getSolrHostColonPort() + path; - solrServer = new HttpSolrClient.Builder(urlString).build(); - - rootDataverseName = findRootDataverseCached().getName(); - } - - @PreDestroy - public void close() { - if (solrServer != null) { - try { - solrServer.close(); - } catch (IOException e) { - logger.warning("Solr closing error: " + e); - } - solrServer = null; - } - } - @TransactionAttribute(REQUIRES_NEW) public Future indexDataverseInNewTransaction(Dataverse dataverse) throws SolrServerException, IOException{ return indexDataverse(dataverse, false); @@ -303,7 +292,7 @@ public Future indexDataverse(Dataverse dataverse, boolean processPaths) String status; try { if (dataverse.getId() != null) { - solrClientService.getSolrClient().add(docs); + solrClientIndexService.getSolrClient().add(docs); } else { logger.info("WARNING: indexing of a dataverse with no id attempted"); } @@ -313,7 +302,7 @@ public Future indexDataverse(Dataverse dataverse, boolean processPaths) return new AsyncResult<>(status); } try { - solrClientService.getSolrClient().commit(); + solrClientIndexService.getSolrClient().commit(); } catch (SolrServerException | IOException ex) { status = ex.toString(); logger.info(status); @@ -1447,8 +1436,8 @@ private String addOrUpdateDataset(IndexableDataset indexableDataset, Set d final SolrInputDocuments docs = toSolrDocs(indexableDataset, datafilesInDraftVersion); try { - solrClientService.getSolrClient().add(docs.getDocuments()); - solrClientService.getSolrClient().commit(); + solrClientIndexService.getSolrClient().add(docs.getDocuments()); + solrClientIndexService.getSolrClient().commit(); } catch (SolrServerException | IOException ex) { if (ex.getCause() instanceof SolrServerException) { throw new SolrServerException(ex); @@ -1689,8 +1678,8 @@ private void updatePathForExistingSolrDocs(DvObject object) throws SolrServerExc sid.removeField(SearchFields.SUBTREE); sid.addField(SearchFields.SUBTREE, paths); - UpdateResponse addResponse = solrClientService.getSolrClient().add(sid); - UpdateResponse commitResponse = solrClientService.getSolrClient().commit(); + UpdateResponse addResponse = solrClientIndexService.getSolrClient().add(sid); + UpdateResponse commitResponse = solrClientIndexService.getSolrClient().commit(); if (object.isInstanceofDataset()) { for (DataFile df : dataset.getFiles()) { solrQuery.setQuery(SearchUtil.constructQuery(SearchFields.ENTITY_ID, df.getId().toString())); @@ -1703,8 +1692,8 @@ private void updatePathForExistingSolrDocs(DvObject object) throws SolrServerExc } sid.removeField(SearchFields.SUBTREE); sid.addField(SearchFields.SUBTREE, paths); - addResponse = solrClientService.getSolrClient().add(sid); - commitResponse = solrClientService.getSolrClient().commit(); + addResponse = solrClientIndexService.getSolrClient().add(sid); + commitResponse = solrClientIndexService.getSolrClient().commit(); } } } @@ -1746,12 +1735,12 @@ public String delete(Dataverse doomed) { logger.fine("deleting Solr document for dataverse " + doomed.getId()); UpdateResponse updateResponse; try { - updateResponse = solrClientService.getSolrClient().deleteById(solrDocIdentifierDataverse + doomed.getId()); + updateResponse = solrClientIndexService.getSolrClient().deleteById(solrDocIdentifierDataverse + doomed.getId()); } catch (SolrServerException | IOException ex) { return ex.toString(); } try { - solrClientService.getSolrClient().commit(); + solrClientIndexService.getSolrClient().commit(); } catch (SolrServerException | IOException ex) { return ex.toString(); } @@ -1771,12 +1760,12 @@ public String removeSolrDocFromIndex(String doomed) { logger.fine("deleting Solr document: " + doomed); UpdateResponse updateResponse; try { - updateResponse = solrClientService.getSolrClient().deleteById(doomed); + updateResponse = solrClientIndexService.getSolrClient().deleteById(doomed); } catch (SolrServerException | IOException ex) { return ex.toString(); } try { - solrClientService.getSolrClient().commit(); + solrClientIndexService.getSolrClient().commit(); } catch (SolrServerException | IOException ex) { return ex.toString(); } @@ -1968,7 +1957,7 @@ public List findPermissionsInSolrOnly() throws SearchException { boolean done = false; while (!done) { q.set(CursorMarkParams.CURSOR_MARK_PARAM, cursorMark); - QueryResponse rsp = solrServer.query(q); + QueryResponse rsp = solrClientService.getSolrClient().query(q); String nextCursorMark = rsp.getNextCursorMark(); SolrDocumentList list = rsp.getResults(); for (SolrDocument doc: list) { @@ -2003,7 +1992,7 @@ private List findDvObjectInSolrOnly(String type) throws SearchException solrQuery.set(CursorMarkParams.CURSOR_MARK_PARAM, cursorMark); QueryResponse rsp = null; try { - rsp = solrServer.query(solrQuery); + rsp = solrClientService.getSolrClient().query(solrQuery); } catch (SolrServerException | IOException ex) { throw new SearchException("Error searching Solr type: " + type, ex); diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java new file mode 100644 index 00000000000..6a98c7cd4a0 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java @@ -0,0 +1,46 @@ +package edu.harvard.iq.dataverse.search; + +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.impl.ConcurrentUpdateHttp2SolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; + +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.ejb.Singleton; +import jakarta.inject.Named; +import java.util.logging.Logger; + +/** + * Solr client to provide insert/update/delete operations. + * Don't use this service with queries to Solr, use SolrClientService instend. + */ +@Named +@Singleton +public class SolrClientIndexService extends AbstractSolrClientService { + private static final Logger logger = Logger.getLogger(SolrClientIndexService.class.getCanonicalName()); + + private SolrClient solrClient; + + @PostConstruct + public void init() { + solrClient = new ConcurrentUpdateHttp2SolrClient.Builder( + getSolrUrl(), new Http2SolrClient.Builder().build()).build(); + } + + @PreDestroy + public void close() { + close(solrClient); + } + + public SolrClient getSolrClient() { + // Should never happen - but? + if (solrClient == null) { + init(); + } + return solrClient; + } + + public void setSolrClient(SolrClient solrClient) { + this.solrClient = solrClient; + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java index b36130de7c8..60ab269455a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java @@ -5,24 +5,20 @@ */ package edu.harvard.iq.dataverse.search; -import edu.harvard.iq.dataverse.settings.JvmSettings; -import edu.harvard.iq.dataverse.util.SystemConfig; import org.apache.solr.client.solrj.SolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; -import jakarta.ejb.EJB; import jakarta.ejb.Singleton; import jakarta.inject.Named; -import java.io.IOException; import java.util.logging.Logger; /** * * @author landreev * - * This singleton is dedicated to initializing the HttpSolrClient used by the + * This singleton is dedicated to initializing the Http2SolrClient used by the * application to talk to the search engine, and serving it to all the other * classes that need it. * This ensures that we are using one client only - as recommended by the @@ -30,36 +26,19 @@ */ @Named @Singleton -public class SolrClientService { +public class SolrClientService extends AbstractSolrClientService { private static final Logger logger = Logger.getLogger(SolrClientService.class.getCanonicalName()); - @EJB - SystemConfig systemConfig; - private SolrClient solrClient; @PostConstruct public void init() { - // Get from MPCONFIG. Might be configured by a sysadmin or simply return the default shipped with - // resources/META-INF/microprofile-config.properties. - String protocol = JvmSettings.SOLR_PROT.lookup(); - String path = JvmSettings.SOLR_PATH.lookup(); - - String urlString = protocol + "://" + systemConfig.getSolrHostColonPort() + path; - solrClient = new HttpSolrClient.Builder(urlString).build(); + solrClient = new Http2SolrClient.Builder(getSolrUrl()).build(); } @PreDestroy public void close() { - if (solrClient != null) { - try { - solrClient.close(); - } catch (IOException e) { - logger.warning("Solr closing error: " + e); - } - - solrClient = null; - } + close(solrClient); } public SolrClient getSolrClient() { @@ -73,9 +52,4 @@ public SolrClient getSolrClient() { public void setSolrClient(SolrClient solrClient) { this.solrClient = solrClient; } - - public void reInitialize() { - close(); - init(); - } } diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java index 04021eb75b6..dfc1ced7c12 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java @@ -46,9 +46,7 @@ public class SolrIndexServiceBean { @EJB DataverseRoleServiceBean rolesSvc; @EJB - IndexServiceBean indexService; - @EJB - SolrClientService solrClientService; + SolrClientIndexService solrClientService; public static String numRowsClearedByClearAllIndexTimes = "numRowsClearedByClearAllIndexTimes"; public static String messageString = "message"; diff --git a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java index adf48e05f09..a9ef468d163 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java @@ -1,16 +1,26 @@ package edu.harvard.iq.dataverse.search; -import edu.harvard.iq.dataverse.*; +import edu.harvard.iq.dataverse.ControlledVocabularyValue; +import edu.harvard.iq.dataverse.DOIServiceBean; +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetField; +import edu.harvard.iq.dataverse.DatasetFieldCompoundValue; +import edu.harvard.iq.dataverse.DatasetFieldConstant; +import edu.harvard.iq.dataverse.DatasetFieldServiceBean; +import edu.harvard.iq.dataverse.DatasetFieldType; +import edu.harvard.iq.dataverse.DatasetFieldValue; +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.Dataverse.DataverseType; +import edu.harvard.iq.dataverse.DataverseServiceBean; +import edu.harvard.iq.dataverse.GlobalId; +import edu.harvard.iq.dataverse.MetadataBlock; import edu.harvard.iq.dataverse.branding.BrandingUtil; import edu.harvard.iq.dataverse.mocks.MocksFactory; -import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.SystemConfig; -import edu.harvard.iq.dataverse.util.testing.JvmSetting; import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.common.SolrInputDocument; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -21,11 +31,15 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.io.IOException; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; +import java.util.Optional; +import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @LocalJvmSettings @@ -40,7 +54,7 @@ public class IndexServiceBeanTest { private SettingsServiceBean settingsService; @InjectMocks private SystemConfig systemConfig = new SystemConfig(); - + @BeforeEach public void setUp() { dataverse = MocksFactory.makeDataverse(); @@ -54,36 +68,6 @@ public void setUp() { Mockito.when(indexService.dataverseService.findRootDataverse()).thenReturn(dataverse); } - - @Test - public void testInitWithDefaults() { - // given - String url = "http://localhost:8983/solr/collection1"; - - // when - indexService.init(); - - // then - HttpSolrClient client = (HttpSolrClient) indexService.solrServer; - assertEquals(url, client.getBaseURL()); - } - - - @Test - @JvmSetting(key = JvmSettings.SOLR_HOST, value = "foobar") - @JvmSetting(key = JvmSettings.SOLR_PORT, value = "1234") - @JvmSetting(key = JvmSettings.SOLR_CORE, value = "test") - void testInitWithConfig() { - // given - String url = "http://foobar:1234/solr/test"; - - // when - indexService.init(); - - // then - HttpSolrClient client = (HttpSolrClient) indexService.solrServer; - assertEquals(url, client.getBaseURL()); - } @Test public void TestIndexing() throws SolrServerException, IOException { @@ -125,6 +109,7 @@ public void testValidateBoundingBox() throws SolrServerException, IOException { assertTrue(!doc.get().containsKey("geolocation")); assertTrue(!doc.get().containsKey("boundingBox")); } + private DatasetField constructBoundingBoxValue(String datasetFieldTypeName, String value) { DatasetField retVal = new DatasetField(); retVal.setDatasetFieldType(new DatasetFieldType(datasetFieldTypeName, DatasetFieldType.FieldType.TEXT, false)); diff --git a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java new file mode 100644 index 00000000000..d6459626faf --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java @@ -0,0 +1,42 @@ +package edu.harvard.iq.dataverse.search; + +import edu.harvard.iq.dataverse.settings.SettingsServiceBean; +import edu.harvard.iq.dataverse.util.SystemConfig; +import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; + +import org.apache.solr.client.solrj.impl.ConcurrentUpdateHttp2SolrClient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +@LocalJvmSettings +@ExtendWith(MockitoExtension.class) +class SolrClientIndexServiceTest { + + @Mock + SettingsServiceBean settingsServiceBean; + @InjectMocks + SystemConfig systemConfig; + SolrClientIndexService clientService = new SolrClientIndexService(); + + @BeforeEach + void setUp() { + clientService.systemConfig = systemConfig; + } + + @Test + void testInitWithDefaults() { + // when + clientService.init(); + + // then + ConcurrentUpdateHttp2SolrClient client = (ConcurrentUpdateHttp2SolrClient) clientService.getSolrClient(); + assertNotNull(client); + } + +} \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java index 72eafcd763c..ebb29e688bd 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java @@ -5,7 +5,7 @@ import edu.harvard.iq.dataverse.util.SystemConfig; import edu.harvard.iq.dataverse.util.testing.JvmSetting; import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; -import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.impl.Http2SolrClient; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -39,7 +39,7 @@ void testInitWithDefaults() { clientService.init(); // then - HttpSolrClient client = (HttpSolrClient) clientService.getSolrClient(); + Http2SolrClient client = (Http2SolrClient) clientService.getSolrClient(); assertEquals(url, client.getBaseURL()); } @@ -55,7 +55,7 @@ void testInitWithConfig() { clientService.init(); // then - HttpSolrClient client = (HttpSolrClient) clientService.getSolrClient(); + Http2SolrClient client = (Http2SolrClient) clientService.getSolrClient(); assertEquals(url, client.getBaseURL()); } } \ No newline at end of file From 65a9871d138f25b6fa12078a1bfd3998658bb53c Mon Sep 17 00:00:00 2001 From: stevenferey Date: Wed, 27 Mar 2024 11:29:34 +0100 Subject: [PATCH 2/9] 10161 - adaptation of Java imports --- .../java/edu/harvard/iq/dataverse/search/IndexServiceBean.java | 1 + .../edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java index f032142cf09..e12ee111a18 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java @@ -10,6 +10,7 @@ import edu.harvard.iq.dataverse.DatasetFieldConstant; import edu.harvard.iq.dataverse.DatasetFieldServiceBean; import edu.harvard.iq.dataverse.DatasetFieldType; +import edu.harvard.iq.dataverse.DatasetFieldValue; import edu.harvard.iq.dataverse.DatasetFieldValueValidator; import edu.harvard.iq.dataverse.DatasetLinkingServiceBean; import edu.harvard.iq.dataverse.DatasetServiceBean; diff --git a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java index bd73f05fea7..cf85899121c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java @@ -1,7 +1,6 @@ package edu.harvard.iq.dataverse.search; import edu.harvard.iq.dataverse.ControlledVocabularyValue; -import edu.harvard.iq.dataverse.DOIServiceBean; import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetField; import edu.harvard.iq.dataverse.DatasetFieldCompoundValue; From 1eceaeef0ac2e2f435e8238c9804d67d4d636f4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Wed, 26 Jun 2024 18:15:02 +0200 Subject: [PATCH 3/9] fix compilation failure after merge --- .../java/edu/harvard/iq/dataverse/search/IndexServiceBean.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java index 0c594653870..6e593eb223b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java @@ -25,6 +25,7 @@ import edu.harvard.iq.dataverse.FileMetadata; import edu.harvard.iq.dataverse.GlobalId; import edu.harvard.iq.dataverse.PermissionServiceBean; +import edu.harvard.iq.dataverse.Retention; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinUserServiceBean; import edu.harvard.iq.dataverse.batch.util.LoggingUtil; From 022b862e04f714b8ff9aed2046cea2277107173f Mon Sep 17 00:00:00 2001 From: Ludovic DANIEL Date: Thu, 5 Sep 2024 15:30:28 +0200 Subject: [PATCH 4/9] Add missing imports after merge --- .../java/edu/harvard/iq/dataverse/search/IndexServiceBean.java | 1 + .../edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java index cfdb20c7e6b..168517c0ebf 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java @@ -16,6 +16,7 @@ import edu.harvard.iq.dataverse.DatasetServiceBean; import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.DatasetVersion.VersionState; +import edu.harvard.iq.dataverse.DatasetVersionServiceBean; import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.DataverseLinkingServiceBean; import edu.harvard.iq.dataverse.DataverseServiceBean; diff --git a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java index f9373f7dd78..7af466021de 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java @@ -9,6 +9,7 @@ import edu.harvard.iq.dataverse.DatasetFieldType; import edu.harvard.iq.dataverse.DatasetFieldValue; import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.DatasetVersionServiceBean; import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.Dataverse.DataverseType; import edu.harvard.iq.dataverse.DataverseServiceBean; From 490a6d219f4fceb9c8b6dd331506d210b7923594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Fri, 25 Oct 2024 18:20:06 +0200 Subject: [PATCH 5/9] Activation via feature-flag --- .../search/AbstractSolrClientService.java | 5 +++ .../search/SolrClientIndexService.java | 14 ++++++-- .../dataverse/search/SolrClientService.java | 19 +++++----- .../iq/dataverse/settings/FeatureFlags.java | 8 ++++- .../search/SolrClientIndexServiceTest.java | 34 +++++++++++++++++- .../search/SolrClientServiceTest.java | 35 ++++++++++++------- 6 files changed, 89 insertions(+), 26 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java b/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java index f36c4a9e591..1ae236d348f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/AbstractSolrClientService.java @@ -9,6 +9,11 @@ import edu.harvard.iq.dataverse.util.SystemConfig; import jakarta.ejb.EJB; +/** + * Generics methods for Solr clients implementations + * + * @author jeromeroucou + */ public abstract class AbstractSolrClientService { private static final Logger logger = Logger.getLogger(AbstractSolrClientService.class.getCanonicalName()); diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java index 6a98c7cd4a0..59281d5fa4e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java @@ -3,7 +3,9 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.ConcurrentUpdateHttp2SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import jakarta.ejb.Singleton; @@ -12,7 +14,7 @@ /** * Solr client to provide insert/update/delete operations. - * Don't use this service with queries to Solr, use SolrClientService instend. + * Don't use this service with queries to Solr, use {@link SolrClientService} instead. */ @Named @Singleton @@ -23,8 +25,14 @@ public class SolrClientIndexService extends AbstractSolrClientService { @PostConstruct public void init() { - solrClient = new ConcurrentUpdateHttp2SolrClient.Builder( - getSolrUrl(), new Http2SolrClient.Builder().build()).build(); + if (FeatureFlags.ENABLE_HTTP2_SOLR_CLIENT.enabled()) { + solrClient = new ConcurrentUpdateHttp2SolrClient.Builder( + getSolrUrl(), new Http2SolrClient.Builder().build()).build(); + } else { + // ConcurrentUpdateSolrClient seem to be more suitable, but + // actually only HttpSolrClient is used. + solrClient = new HttpSolrClient.Builder(getSolrUrl()).build(); + } } @PreDestroy diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java index 60ab269455a..83f16e29af2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java @@ -1,13 +1,10 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ package edu.harvard.iq.dataverse.search; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; +import edu.harvard.iq.dataverse.settings.FeatureFlags; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import jakarta.ejb.Singleton; @@ -18,9 +15,9 @@ * * @author landreev * - * This singleton is dedicated to initializing the Http2SolrClient used by the - * application to talk to the search engine, and serving it to all the other - * classes that need it. + * This singleton is dedicated to initializing the HttpSolrClient, or the Http2SolrClient + * (if feature-flag is enabled), used by the application to talk to the search engine, + * and serving it to all the other classes that need it. * This ensures that we are using one client only - as recommended by the * documentation. */ @@ -33,7 +30,11 @@ public class SolrClientService extends AbstractSolrClientService { @PostConstruct public void init() { - solrClient = new Http2SolrClient.Builder(getSolrUrl()).build(); + if (FeatureFlags.ENABLE_HTTP2_SOLR_CLIENT.enabled()) { + solrClient = new Http2SolrClient.Builder(getSolrUrl()).build(); + } else { + solrClient = new HttpSolrClient.Builder(getSolrUrl()).build(); + } } @PreDestroy diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java index 33e828e619d..97a8eae1d9c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java @@ -67,7 +67,13 @@ public enum FeatureFlags { * @since Dataverse 6.3 */ INDEX_HARVESTED_METADATA_SOURCE("index-harvested-metadata-source"), - + /** + * With this flag enabled, a the new Solr client Http2SolrClient is used in + * order to replace HttpSolrClient witch is deprecated since Solr 9. + * + * @apiNote Raise flag by setting "dataverse.feature.enable-http2-solr-client" + */ + ENABLE_HTTP2_SOLR_CLIENT("enable-http2-solr-client"), /** * Dataverse normally deletes all solr documents related to a dataset's files * when the dataset is reindexed. With this flag enabled, additional logic is diff --git a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java index d6459626faf..09d9e096e55 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java @@ -1,10 +1,14 @@ package edu.harvard.iq.dataverse.search; +import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.SystemConfig; +import edu.harvard.iq.dataverse.util.testing.JvmSetting; import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; +import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.ConcurrentUpdateHttp2SolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -12,16 +16,21 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; + @LocalJvmSettings @ExtendWith(MockitoExtension.class) class SolrClientIndexServiceTest { @Mock SettingsServiceBean settingsServiceBean; + @InjectMocks SystemConfig systemConfig; + SolrClientIndexService clientService = new SolrClientIndexService(); @BeforeEach @@ -31,12 +40,35 @@ void setUp() { @Test void testInitWithDefaults() { + // given + String url = "http://localhost:8983/solr/collection1"; + // when clientService.init(); // then - ConcurrentUpdateHttp2SolrClient client = (ConcurrentUpdateHttp2SolrClient) clientService.getSolrClient(); + SolrClient client = clientService.getSolrClient(); assertNotNull(client); + assertInstanceOf(HttpSolrClient.class, client); + assertEquals(url, clientService.getSolrUrl()); } + @Test + @JvmSetting(key = JvmSettings.SOLR_HOST, value = "foobar") + @JvmSetting(key = JvmSettings.SOLR_PORT, value = "1234") + @JvmSetting(key = JvmSettings.SOLR_CORE, value = "test") + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "on", varArgs = "enable-http2-solr-client") + void testInitWithConfig() { + // given + String url = "http://foobar:1234/solr/test"; + + // when + clientService.init(); + + // then + SolrClient client = clientService.getSolrClient(); + assertNotNull(client); + assertInstanceOf(ConcurrentUpdateHttp2SolrClient.class, client); + assertEquals(url, clientService.getSolrUrl()); + } } \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java index ebb29e688bd..351d553f203 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java @@ -5,7 +5,10 @@ import edu.harvard.iq.dataverse.util.SystemConfig; import edu.harvard.iq.dataverse.util.testing.JvmSetting; import edu.harvard.iq.dataverse.util.testing.LocalJvmSettings; + +import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -14,48 +17,56 @@ import org.mockito.junit.jupiter.MockitoExtension; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; + @LocalJvmSettings @ExtendWith(MockitoExtension.class) class SolrClientServiceTest { - + @Mock SettingsServiceBean settingsServiceBean; @InjectMocks SystemConfig systemConfig; SolrClientService clientService = new SolrClientService(); - + @BeforeEach void setUp() { clientService.systemConfig = systemConfig; } - + @Test void testInitWithDefaults() { // given String url = "http://localhost:8983/solr/collection1"; - + // when clientService.init(); - + // then - Http2SolrClient client = (Http2SolrClient) clientService.getSolrClient(); - assertEquals(url, client.getBaseURL()); + SolrClient client = clientService.getSolrClient(); + assertNotNull(client); + assertInstanceOf(HttpSolrClient.class, client); + assertEquals(url, ((HttpSolrClient) client).getBaseURL()); } - + @Test @JvmSetting(key = JvmSettings.SOLR_HOST, value = "foobar") @JvmSetting(key = JvmSettings.SOLR_PORT, value = "1234") @JvmSetting(key = JvmSettings.SOLR_CORE, value = "test") + @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "on", varArgs = "enable-http2-solr-client") void testInitWithConfig() { // given String url = "http://foobar:1234/solr/test"; - + // when clientService.init(); - + // then - Http2SolrClient client = (Http2SolrClient) clientService.getSolrClient(); - assertEquals(url, client.getBaseURL()); + SolrClient client = clientService.getSolrClient(); + assertNotNull(client); + assertInstanceOf(Http2SolrClient.class, client); + assertEquals(url, clientService.getSolrUrl()); } } \ No newline at end of file From 728efffc814bf04a94e6350ee14b7ec0a3889576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Fri, 25 Oct 2024 18:21:04 +0200 Subject: [PATCH 6/9] Documentations --- doc/release-notes/10241-new-solr-client.md | 9 +++++++++ doc/sphinx-guides/source/installation/config.rst | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 doc/release-notes/10241-new-solr-client.md diff --git a/doc/release-notes/10241-new-solr-client.md b/doc/release-notes/10241-new-solr-client.md new file mode 100644 index 00000000000..211ad9d1ec9 --- /dev/null +++ b/doc/release-notes/10241-new-solr-client.md @@ -0,0 +1,9 @@ +[HttpSolrClient](https://solr.apache.org/docs/9_3_0/solrj/org/apache/solr/client/solrj/impl/HttpSolrClient.html) is deprecated as of Solr 9, and which will be removed in a future major release of Solr. It's recommended to use [Http2SolrClient](https://solr.apache.org/docs/9_3_0/solrj/org/apache/solr/client/solrj/impl/Http2SolrClient.html) instead. + +[Solr documentation](https://solr.apache.org/guide/solr/latest/deployment-guide/solrj.html#types-of-solrclients) describe it as a _async, non-blocking and general-purpose client that leverage HTTP/2 using the Jetty Http library_. + +With Solr 9.3.0, the Http2SolrClient is indicate as experimental. But since the 9.6 version of Solr, this mention is no longer maintained. + +For the time being, its activation is therefore conditional on the use of a [feature-flag](https://dataverse-guide--10241.org.readthedocs.build/en/10241/installation/config.html#feature-flags). Activation also enables ConcurrentUpdateHttp2SolrClient, which is supposed to be more efficient for indexing. + +For more information, see issue [#10161](https://github.com/IQSS/dataverse/issues/10161) and pull request [#10241](https://github.com/IQSS/dataverse/pull/10241) diff --git a/doc/sphinx-guides/source/installation/config.rst b/doc/sphinx-guides/source/installation/config.rst index e98ed8f5189..9823e3b6432 100644 --- a/doc/sphinx-guides/source/installation/config.rst +++ b/doc/sphinx-guides/source/installation/config.rst @@ -3345,8 +3345,8 @@ please find all known feature flags below. Any of these flags can be activated u * - add-publicobject-solr-field - Adds an extra boolean field `PublicObject_b:true` for public content (published Collections, Datasets and Files). Once reindexed with these fields, we can rely on it to remove a very expensive Solr join on all such documents in Solr queries, significantly improving overall performance (by enabling the feature flag above, `avoid-expensive-solr-join`). These two flags are separate so that an instance can reindex their holdings before enabling the optimization in searches, thus avoiding having their public objects temporarily disappear from search results while the reindexing is in progress. - ``Off`` - * - reduce-solr-deletes - - Avoids deleting and recreating solr documents for dataset files when reindexing. + * - enable-http2-solr-client + - Enable a new Solr client ``Http2SolrClient`` instead of ``HttpSolrClient`` witch is deprecated since Solr 9. Also enables use of ``ConcurrentUpdateHttp2SolrClient``, recommended to send concurrent updates to Solr. More informations about this Solr clients on `Solr documentation `_. - ``Off`` * - reduce-solr-deletes - Avoids deleting and recreating solr documents for dataset files when reindexing. From 3adb19419792d9304a51e75e76619305b568aa3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Fri, 15 Nov 2024 17:51:36 +0100 Subject: [PATCH 7/9] revert feature-flag --- .../search/SolrClientIndexService.java | 17 ++++++----------- .../iq/dataverse/search/SolrClientService.java | 14 ++++---------- .../iq/dataverse/settings/FeatureFlags.java | 7 ------- .../search/SolrClientIndexServiceTest.java | 4 +--- .../dataverse/search/SolrClientServiceTest.java | 6 ++---- 5 files changed, 13 insertions(+), 35 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java index 59281d5fa4e..0b7f1aae798 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientIndexService.java @@ -1,16 +1,15 @@ package edu.harvard.iq.dataverse.search; +import java.util.logging.Logger; + import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.ConcurrentUpdateHttp2SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient; -import edu.harvard.iq.dataverse.settings.FeatureFlags; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import jakarta.ejb.Singleton; import jakarta.inject.Named; -import java.util.logging.Logger; /** * Solr client to provide insert/update/delete operations. @@ -19,20 +18,15 @@ @Named @Singleton public class SolrClientIndexService extends AbstractSolrClientService { + private static final Logger logger = Logger.getLogger(SolrClientIndexService.class.getCanonicalName()); private SolrClient solrClient; @PostConstruct public void init() { - if (FeatureFlags.ENABLE_HTTP2_SOLR_CLIENT.enabled()) { - solrClient = new ConcurrentUpdateHttp2SolrClient.Builder( - getSolrUrl(), new Http2SolrClient.Builder().build()).build(); - } else { - // ConcurrentUpdateSolrClient seem to be more suitable, but - // actually only HttpSolrClient is used. - solrClient = new HttpSolrClient.Builder(getSolrUrl()).build(); - } + solrClient = new ConcurrentUpdateHttp2SolrClient.Builder( + getSolrUrl(), new Http2SolrClient.Builder().build()).build(); } @PreDestroy @@ -51,4 +45,5 @@ public SolrClient getSolrClient() { public void setSolrClient(SolrClient solrClient) { this.solrClient = solrClient; } + } diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java index 83f16e29af2..f9d94b8c6d3 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java @@ -2,9 +2,7 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient; -import edu.harvard.iq.dataverse.settings.FeatureFlags; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import jakarta.ejb.Singleton; @@ -15,9 +13,9 @@ * * @author landreev * - * This singleton is dedicated to initializing the HttpSolrClient, or the Http2SolrClient - * (if feature-flag is enabled), used by the application to talk to the search engine, - * and serving it to all the other classes that need it. + * This singleton is dedicated to initializing the Http2SolrClient, used by + * the application to talk to the search engine, and serving it to all the + * other classes that need it. * This ensures that we are using one client only - as recommended by the * documentation. */ @@ -30,11 +28,7 @@ public class SolrClientService extends AbstractSolrClientService { @PostConstruct public void init() { - if (FeatureFlags.ENABLE_HTTP2_SOLR_CLIENT.enabled()) { - solrClient = new Http2SolrClient.Builder(getSolrUrl()).build(); - } else { - solrClient = new HttpSolrClient.Builder(getSolrUrl()).build(); - } + solrClient = new Http2SolrClient.Builder(getSolrUrl()).build(); } @PreDestroy diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java index 97a8eae1d9c..24b41c623b6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java @@ -67,13 +67,6 @@ public enum FeatureFlags { * @since Dataverse 6.3 */ INDEX_HARVESTED_METADATA_SOURCE("index-harvested-metadata-source"), - /** - * With this flag enabled, a the new Solr client Http2SolrClient is used in - * order to replace HttpSolrClient witch is deprecated since Solr 9. - * - * @apiNote Raise flag by setting "dataverse.feature.enable-http2-solr-client" - */ - ENABLE_HTTP2_SOLR_CLIENT("enable-http2-solr-client"), /** * Dataverse normally deletes all solr documents related to a dataset's files * when the dataset is reindexed. With this flag enabled, additional logic is diff --git a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java index 09d9e096e55..d3d68fa5f6a 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientIndexServiceTest.java @@ -8,7 +8,6 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.ConcurrentUpdateHttp2SolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -49,7 +48,7 @@ void testInitWithDefaults() { // then SolrClient client = clientService.getSolrClient(); assertNotNull(client); - assertInstanceOf(HttpSolrClient.class, client); + assertInstanceOf(ConcurrentUpdateHttp2SolrClient.class, client); assertEquals(url, clientService.getSolrUrl()); } @@ -57,7 +56,6 @@ void testInitWithDefaults() { @JvmSetting(key = JvmSettings.SOLR_HOST, value = "foobar") @JvmSetting(key = JvmSettings.SOLR_PORT, value = "1234") @JvmSetting(key = JvmSettings.SOLR_CORE, value = "test") - @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "on", varArgs = "enable-http2-solr-client") void testInitWithConfig() { // given String url = "http://foobar:1234/solr/test"; diff --git a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java index 351d553f203..13cea4151ff 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/SolrClientServiceTest.java @@ -8,7 +8,6 @@ import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.impl.Http2SolrClient; -import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -47,15 +46,14 @@ void testInitWithDefaults() { // then SolrClient client = clientService.getSolrClient(); assertNotNull(client); - assertInstanceOf(HttpSolrClient.class, client); - assertEquals(url, ((HttpSolrClient) client).getBaseURL()); + assertInstanceOf(Http2SolrClient.class, client); + assertEquals(url, clientService.getSolrUrl()); } @Test @JvmSetting(key = JvmSettings.SOLR_HOST, value = "foobar") @JvmSetting(key = JvmSettings.SOLR_PORT, value = "1234") @JvmSetting(key = JvmSettings.SOLR_CORE, value = "test") - @JvmSetting(key = JvmSettings.FEATURE_FLAG, value = "on", varArgs = "enable-http2-solr-client") void testInitWithConfig() { // given String url = "http://foobar:1234/solr/test"; From 6a05ecdcbbccae3fcc54711ba5a7b76dbd44049d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Fri, 15 Nov 2024 18:05:42 +0100 Subject: [PATCH 8/9] revert feature flag documentation --- doc/release-notes/10241-new-solr-client.md | 6 +++--- doc/sphinx-guides/source/installation/config.rst | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/doc/release-notes/10241-new-solr-client.md b/doc/release-notes/10241-new-solr-client.md index 211ad9d1ec9..67ccdd4f184 100644 --- a/doc/release-notes/10241-new-solr-client.md +++ b/doc/release-notes/10241-new-solr-client.md @@ -1,9 +1,9 @@ -[HttpSolrClient](https://solr.apache.org/docs/9_3_0/solrj/org/apache/solr/client/solrj/impl/HttpSolrClient.html) is deprecated as of Solr 9, and which will be removed in a future major release of Solr. It's recommended to use [Http2SolrClient](https://solr.apache.org/docs/9_3_0/solrj/org/apache/solr/client/solrj/impl/Http2SolrClient.html) instead. +[HttpSolrClient](https://solr.apache.org/docs/9_4_1/solrj/org/apache/solr/client/solrj/impl/HttpSolrClient.html) is deprecated as of Solr 9, and which will be removed in a future major release of Solr. It's recommended to use [Http2SolrClient](https://solr.apache.org/docs/9_4_1/solrj/org/apache/solr/client/solrj/impl/Http2SolrClient.html) instead. [Solr documentation](https://solr.apache.org/guide/solr/latest/deployment-guide/solrj.html#types-of-solrclients) describe it as a _async, non-blocking and general-purpose client that leverage HTTP/2 using the Jetty Http library_. -With Solr 9.3.0, the Http2SolrClient is indicate as experimental. But since the 9.6 version of Solr, this mention is no longer maintained. +With Solr 9.4.1, the Http2SolrClient is indicate as experimental. But since the 9.6 version of Solr, this mention is no longer maintained. -For the time being, its activation is therefore conditional on the use of a [feature-flag](https://dataverse-guide--10241.org.readthedocs.build/en/10241/installation/config.html#feature-flags). Activation also enables ConcurrentUpdateHttp2SolrClient, which is supposed to be more efficient for indexing. +The ConcurrentUpdateHttp2SolrClient is now also used in some cases, which is supposed to be more efficient for indexing. For more information, see issue [#10161](https://github.com/IQSS/dataverse/issues/10161) and pull request [#10241](https://github.com/IQSS/dataverse/pull/10241) diff --git a/doc/sphinx-guides/source/installation/config.rst b/doc/sphinx-guides/source/installation/config.rst index 9823e3b6432..b4705aa6521 100644 --- a/doc/sphinx-guides/source/installation/config.rst +++ b/doc/sphinx-guides/source/installation/config.rst @@ -3345,9 +3345,6 @@ please find all known feature flags below. Any of these flags can be activated u * - add-publicobject-solr-field - Adds an extra boolean field `PublicObject_b:true` for public content (published Collections, Datasets and Files). Once reindexed with these fields, we can rely on it to remove a very expensive Solr join on all such documents in Solr queries, significantly improving overall performance (by enabling the feature flag above, `avoid-expensive-solr-join`). These two flags are separate so that an instance can reindex their holdings before enabling the optimization in searches, thus avoiding having their public objects temporarily disappear from search results while the reindexing is in progress. - ``Off`` - * - enable-http2-solr-client - - Enable a new Solr client ``Http2SolrClient`` instead of ``HttpSolrClient`` witch is deprecated since Solr 9. Also enables use of ``ConcurrentUpdateHttp2SolrClient``, recommended to send concurrent updates to Solr. More informations about this Solr clients on `Solr documentation `_. - - ``Off`` * - reduce-solr-deletes - Avoids deleting and recreating solr documents for dataset files when reindexing. - ``Off`` From 7cae923c2d9143006ad8f39ceb0329644cd73643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20ROUCOU?= Date: Wed, 22 Jan 2025 11:04:47 +0100 Subject: [PATCH 9/9] Fix missing import --- .../java/edu/harvard/iq/dataverse/search/IndexServiceBean.java | 1 + .../edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java index c4673f79b07..839dd4a7e08 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/IndexServiceBean.java @@ -16,6 +16,7 @@ import edu.harvard.iq.dataverse.DatasetServiceBean; import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.DatasetVersion.VersionState; +import edu.harvard.iq.dataverse.DatasetVersionFilesServiceBean; import edu.harvard.iq.dataverse.DatasetVersionServiceBean; import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.DataverseLinkingServiceBean; diff --git a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java index 54b9ed4888a..2b54a4b12cd 100644 --- a/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/search/IndexServiceBeanTest.java @@ -9,6 +9,7 @@ import edu.harvard.iq.dataverse.DatasetFieldType; import edu.harvard.iq.dataverse.DatasetFieldValue; import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.DatasetVersionFilesServiceBean; import edu.harvard.iq.dataverse.DatasetVersionServiceBean; import edu.harvard.iq.dataverse.Dataverse; import edu.harvard.iq.dataverse.Dataverse.DataverseType;