Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

TASK-6981 - Should not use same Solr timeout for reading and for indexing #2512

Open
wants to merge 6 commits into
base: release-3.x.x
Choose a base branch
from
Open

TASK-6981 - Should not use same Solr timeout for reading and for indexing #2512

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f7cec5
storage: add Solr write timeout in the storage configuration file, #T…
jtarraga Sep 24, 2024
2757197
storage: the variant storage engine uses a Solr client with extended …
jtarraga Sep 27, 2024
c8a00b0
search: update according to java-common-libs changes, and recover the…
jtarraga Sep 30, 2024
68c45d2
Merge branch 'release-3.x.x' into TASK-6981
jtarraga Sep 30, 2024
e2ab75e
storage: add timeout comments in the storage configuration file, #TAS…
jtarraga Oct 7, 2024
abf04bd
storage: add log message, #TASK-6981
jtarraga Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 30 additions & 29 deletions opencga-core/src/main/java/org/opencb/opencga/core/config/SearchConfiguration.java
View file
Original file line number Diff line number Diff line change
@@ -36,20 +36,29 @@ public class SearchConfiguration {
private String manager;
private boolean active;
private int timeout;
private int writeTimeout;
private int insertBatchSize;

private static final String DEFAULT_MODE = "cloud";
private static final boolean DEFAULT_ACTIVE = true;
private static final int DEFAULT_TIMEOUT = 30000;
private static final int DEFAULT_WRITE_TIMEOUT = 120000;
private static final int DEFAULT_INSERT_BATCH_SIZE = 10000;


public SearchConfiguration() {
this(Collections.emptyList(), "", DEFAULT_MODE, "", "", "", DEFAULT_ACTIVE, DEFAULT_TIMEOUT, DEFAULT_INSERT_BATCH_SIZE);
this(Collections.emptyList(), "", DEFAULT_MODE, "", "", "", DEFAULT_ACTIVE, DEFAULT_TIMEOUT, DEFAULT_WRITE_TIMEOUT,
DEFAULT_INSERT_BATCH_SIZE);
}

@Deprecated
public SearchConfiguration(List<String> hosts, String configSet, String mode, String user, String password, String manager,
boolean active, int timeout, int insertBatchSize) {
this(hosts, configSet, mode, user, password, manager, active, timeout, DEFAULT_WRITE_TIMEOUT, insertBatchSize);
}

public SearchConfiguration(List<String> hosts, String configSet, String mode, String user, String password, String manager,
boolean active, int timeout, int writeTimeout, int insertBatchSize) {
this.hosts = hosts;
this.configSet = configSet;
this.mode = mode;
@@ -58,32 +67,25 @@ public SearchConfiguration(List<String> hosts, String configSet, String mode, St
this.manager = manager;
this.active = active;
this.timeout = timeout;
this.writeTimeout = writeTimeout;
this.insertBatchSize = insertBatchSize;
}

@Override
public String toString() {
return "SearchConfiguration{" +
"hosts=" + hosts +
", configSet='" + configSet + '\'' +
", mode='" + mode + '\'' +
", user='" + user + '\'' +
", password='" + password + '\'' +
", manager='" + manager + '\'' +
", active=" + active +
", timeout=" + timeout +
", insertBatchSize=" + insertBatchSize +
'}';
}

@Deprecated
public String getHost() {
return String.join(",", getHosts());
}

@Deprecated
public SearchConfiguration setHost(String host) {
return setHosts(StringUtils.isEmpty(host) ? Collections.emptyList() : Arrays.asList(host.split(",")));
final StringBuilder sb = new StringBuilder("SearchConfiguration{");
sb.append("hosts=").append(hosts);
sb.append(", configSet='").append(configSet).append('\'');
sb.append(", mode='").append(mode).append('\'');
sb.append(", user='").append(user).append('\'');
sb.append(", password='").append(password).append('\'');
sb.append(", manager='").append(manager).append('\'');
sb.append(", active=").append(active);
sb.append(", timeout=").append(timeout);
sb.append(", writeTimeout=").append(writeTimeout);
sb.append(", insertBatchSize=").append(insertBatchSize);
sb.append('}');
return sb.toString();
}

public List<String> getHosts() {
@@ -99,8 +101,9 @@ public String getConfigSet() {
return configSet;
}

public void setConfigSet(String configSet) {
public SearchConfiguration setConfigSet(String configSet) {
this.configSet = configSet;
return this;
}

public String getMode() {
@@ -157,14 +160,12 @@ public SearchConfiguration setTimeout(int timeout) {
return this;
}

@Deprecated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not remove these methods.
Leave the methods deprecated, and just call to the static method
Configuration.reportUnusedField()

e.g.

opencga/opencga-core/src/main/java/org/opencb/opencga/core/config/Configuration.java

Lines 304 to 313 in e7f1966

@Deprecated
public Admin getAdmin() {
return null;
}
@Deprecated
public Configuration setAdmin(Admin admin) {
reportUnusedField("configuration.yml#admin", admin);
return this;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will avoid failures in "old" configuration files that might be using the "rows" or "host" field.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ticket should have, in the "Communications" a message saying that this fields are removed.

public int getRows() {
return insertBatchSize;
public int getWriteTimeout() {
return writeTimeout;
}

@Deprecated
public SearchConfiguration setRows(int rows) {
this.insertBatchSize = rows;
public SearchConfiguration setWriteTimeout(int writeTimeout) {
this.writeTimeout = writeTimeout;
return this;
}

12 changes: 6 additions & 6 deletions ...main/java/org/opencb/opencga/storage/app/cli/client/executors/VariantCommandExecutor.java
View file
Original file line number Diff line number Diff line change
@@ -36,28 +36,28 @@
import org.opencb.commons.utils.FileUtils;
import org.opencb.opencga.core.common.TimeUtils;
import org.opencb.opencga.core.common.UriUtils;
import org.opencb.opencga.core.config.storage.StorageEngineConfiguration;
import org.opencb.opencga.core.models.common.mixins.GenericRecordAvroJsonMixin;
import org.opencb.opencga.core.models.operations.variant.VariantAggregateFamilyParams;
import org.opencb.opencga.core.models.operations.variant.VariantAggregateParams;
import org.opencb.opencga.storage.app.cli.CommandExecutor;
import org.opencb.opencga.storage.app.cli.GeneralCliOptions;
import org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions;
import org.opencb.opencga.storage.core.StorageEngineFactory;
import org.opencb.opencga.core.config.storage.StorageEngineConfiguration;
import org.opencb.opencga.storage.core.exceptions.StorageEngineException;
import org.opencb.opencga.storage.core.metadata.models.ProjectMetadata;
import org.opencb.opencga.storage.core.variant.VariantStorageEngine;
import org.opencb.opencga.storage.core.variant.VariantStorageOptions;
import org.opencb.opencga.storage.core.variant.VariantStoragePipeline;
import org.opencb.opencga.storage.core.variant.adaptors.VariantQueryParam;
import org.opencb.opencga.storage.core.variant.query.VariantQueryUtils;
import org.opencb.opencga.storage.core.variant.adaptors.iterators.VariantDBIterator;
import org.opencb.opencga.storage.core.variant.annotation.DefaultVariantAnnotationManager;
import org.opencb.opencga.storage.core.variant.annotation.VariantAnnotationManager;
import org.opencb.opencga.storage.core.variant.annotation.VariantAnnotatorException;
import org.opencb.opencga.storage.core.variant.io.VariantWriterFactory;
import org.opencb.opencga.core.models.common.mixins.GenericRecordAvroJsonMixin;
import org.opencb.opencga.storage.core.variant.search.solr.VariantSearchManager;
import org.opencb.opencga.storage.core.variant.query.VariantQueryUtils;
import org.opencb.opencga.storage.core.variant.search.solr.SolrVariantDBIterator;
import org.opencb.opencga.storage.core.variant.search.solr.VariantSearchManager;
import org.opencb.opencga.storage.core.variant.stats.DefaultVariantStatisticsManager;

import java.io.*;
@@ -68,8 +68,8 @@
import java.util.*;
import java.util.function.Function;

import static org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions.AggregateFamilyCommandOptions.AGGREGATE_FAMILY_COMMAND;
import static org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions.AggregateCommandOptions.AGGREGATE_COMMAND;
import static org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions.AggregateFamilyCommandOptions.AGGREGATE_FAMILY_COMMAND;
import static org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions.GenericAnnotationDeleteCommandOptions.ANNOTATION_DELETE_COMMAND;
import static org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions.GenericAnnotationMetadataCommandOptions.ANNOTATION_METADATA_COMMAND;
import static org.opencb.opencga.storage.app.cli.client.options.StorageVariantCommandOptions.GenericAnnotationQueryCommandOptions.ANNOTATION_QUERY_COMMAND;
@@ -732,7 +732,7 @@ private void search() throws Exception {
String solrUrl = (searchOptions.solrUrl == null ? "http://localhost:8983/solr/" : searchOptions.solrUrl);
String dbName = (searchOptions.dbName == null ? "variants" : searchOptions.dbName);

variantStorageEngine.getConfiguration().getSearch().setHost(solrUrl);
variantStorageEngine.getConfiguration().getSearch().setHosts(Collections.singletonList(solrUrl));

// VariantSearchManager variantSearchManager = new VariantSearchManager(solrUrl, dbName);
// VariantSearchManager variantSearchManager = new VariantSearchManager(variantStorageEngine.getStudyConfigurationManager(),
4 changes: 2 additions & 2 deletions ...rage-core/src/main/java/org/opencb/opencga/storage/core/variant/VariantStorageEngine.java
View file
Original file line number Diff line number Diff line change
@@ -34,7 +34,6 @@
import org.opencb.opencga.core.models.operations.variant.VariantAggregateFamilyParams;
import org.opencb.opencga.core.models.operations.variant.VariantAggregateParams;
import org.opencb.opencga.core.models.variant.VariantSetupParams;
import org.opencb.opencga.storage.core.variant.query.VariantQueryResult;
import org.opencb.opencga.storage.core.StorageEngine;
import org.opencb.opencga.storage.core.StoragePipelineResult;
import org.opencb.opencga.storage.core.exceptions.StorageEngineException;
@@ -60,6 +59,7 @@
import org.opencb.opencga.storage.core.variant.io.VariantWriterFactory.VariantOutputFormat;
import org.opencb.opencga.storage.core.variant.query.ParsedVariantQuery;
import org.opencb.opencga.storage.core.variant.query.VariantQueryParser;
import org.opencb.opencga.storage.core.variant.query.VariantQueryResult;
import org.opencb.opencga.storage.core.variant.query.VariantQueryUtils;
import org.opencb.opencga.storage.core.variant.query.executors.*;
import org.opencb.opencga.storage.core.variant.score.VariantScoreFormatDescriptor;
@@ -794,7 +794,7 @@ protected void searchIndexLoadedFiles(List<URI> inputFiles, ObjectMap options) t

protected SolrInputDocumentDataWriter newVariantSearchDataWriter(String collection) throws StorageEngineException {
return new SolrInputDocumentDataWriter(collection,
getVariantSearchManager().getSolrClient(),
getVariantSearchManager().getSolrManager().newSolrClient(configuration.getSearch().getWriteTimeout()), true,
getVariantSearchManager().getInsertBatchSize());
}

19 changes: 19 additions & 0 deletions ...java/org/opencb/opencga/storage/core/variant/search/solr/SolrInputDocumentDataWriter.java
View file
Original file line number Diff line number Diff line change
@@ -9,12 +9,14 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.List;

public class SolrInputDocumentDataWriter implements DataWriter<SolrInputDocument> {

private final String collection;
private final SolrClient solrClient;
private boolean closeSolrClient;
private final int insertBatchSize;
private int serverBufferSize = 0;
private int insertedDocuments = 0;
@@ -23,8 +25,13 @@ public class SolrInputDocumentDataWriter implements DataWriter<SolrInputDocument
private final Logger logger = LoggerFactory.getLogger(SolrInputDocumentDataWriter.class);

public SolrInputDocumentDataWriter(String collection, SolrClient solrClient, int insertBatchSize) {
this(collection, solrClient, false, insertBatchSize);
}

public SolrInputDocumentDataWriter(String collection, SolrClient solrClient, boolean closeSolrClient, int insertBatchSize) {
this.collection = collection;
this.solrClient = solrClient;
this.closeSolrClient = closeSolrClient;
this.insertBatchSize = insertBatchSize;
}

@@ -54,6 +61,18 @@ public boolean post() {
return true;
}

@Override
public boolean close() {
if (closeSolrClient) {
try {
solrClient.close();
} catch (IOException e) {
Throwables.propagate(e);
}
}
return true;
}

protected void add(List<SolrInputDocument> batch) throws Exception {
UpdateResponse response = solrClient.add(collection, batch);
addTimeMs += response.getElapsedTime();
1 change: 1 addition & 0 deletions opencga-storage/opencga-storage-core/src/main/resources/storage-configuration.yml
View file
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ search:
configSet: "${OPENCGA.STORAGE.SEARCH.CONFIG_SET}"
mode: "cloud"
timeout: ${OPENCGA.STORAGE.SEARCH.TIMEOUT}
writeTimeout: ${OPENCGA.STORAGE.SEARCH.WRITE.TIMEOUT}
insertBatchSize: 5000

## Clinical database for indexing the pathogenic variants reported.
3 changes: 1 addition & 2 deletions ...c/test/java/org/opencb/opencga/storage/core/variant/solr/VariantSolrExternalResource.java
View file
Original file line number Diff line number Diff line change
@@ -116,8 +116,7 @@ public VariantSearchManager configure(VariantStorageEngine variantStorageEngine)
variantStorageEngine.getConfiguration().getSearch().setMode("core");
variantStorageEngine.getConfiguration().getSearch().setActive(true);
VariantSearchManager variantSearchManager = variantStorageEngine.getVariantSearchManager();
variantSearchManager.setSolrManager(new SolrManager(solrClient, "localhost", "core",
variantStorageEngine.getConfiguration().getSearch().getTimeout()));
variantSearchManager.setSolrManager(new SolrManager(solrClient, "localhost", "core"));
variantSearchManager.setSolrClient(solrClient);
return variantSearchManager;
}
1 change: 1 addition & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
@@ -1397,6 +1397,7 @@
<OPENCGA.STORAGE.SEARCH.CONFIG_SET>opencga-variant-configset-${project.parent.version}
</OPENCGA.STORAGE.SEARCH.CONFIG_SET>
<OPENCGA.STORAGE.SEARCH.TIMEOUT>30000</OPENCGA.STORAGE.SEARCH.TIMEOUT>
<OPENCGA.STORAGE.SEARCH.WRITE.TIMEOUT>120000</OPENCGA.STORAGE.SEARCH.WRITE.TIMEOUT>
<OPENCGA.STORAGE.CLINICAL.HOST>http://localhost:8983/solr/</OPENCGA.STORAGE.CLINICAL.HOST>
<OPENCGA.STORAGE.CLINICAL.MANAGER>""</OPENCGA.STORAGE.CLINICAL.MANAGER>
<OPENCGA.STORAGE.CLINICAL.TIMEOUT>30000</OPENCGA.STORAGE.CLINICAL.TIMEOUT>
Loading