Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

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

Show file tree
Hide file tree
Changes from all 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
60 changes: 42 additions & 18 deletions opencga-core/src/main/java/org/opencb/opencga/core/config/SearchConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.Collections;
import java.util.List;

import static org.opencb.opencga.core.config.Configuration.reportUnusedField;

/**
* Created by wasim on 09/11/16.
*/
Expand All @@ -36,20 +38,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;
Expand All @@ -58,32 +69,36 @@ 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 +
'}';
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();
}

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

@Deprecated
public SearchConfiguration setHost(String host) {
return setHosts(StringUtils.isEmpty(host) ? Collections.emptyList() : Arrays.asList(host.split(",")));
reportUnusedField("search.configuration#host", host);
return this;
}

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

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

public String getMode() {
Expand Down Expand Up @@ -157,17 +173,25 @@ public SearchConfiguration setTimeout(int timeout) {
return this;
}

public int getWriteTimeout() {
return writeTimeout;
}

public SearchConfiguration setWriteTimeout(int writeTimeout) {
this.writeTimeout = writeTimeout;
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;
return 0;
}

@Deprecated
public SearchConfiguration setRows(int rows) {
this.insertBatchSize = rows;
reportUnusedField("search.configuration#rows", rows);
return this;
}

public int getInsertBatchSize() {
return insertBatchSize;
}
Expand Down
12 changes: 6 additions & 6 deletions ...main/java/org/opencb/opencga/storage/app/cli/client/executors/VariantCommandExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 3 additions & 2 deletions ...rage-core/src/main/java/org/opencb/opencga/storage/core/variant/VariantStorageEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -793,8 +793,9 @@ protected void searchIndexLoadedFiles(List<URI> inputFiles, ObjectMap options) t
}

protected SolrInputDocumentDataWriter newVariantSearchDataWriter(String collection) throws StorageEngineException {
logger.info("Using SolrClient with a write timeout of {} ms", configuration.getSearch().getWriteTimeout());
return new SolrInputDocumentDataWriter(collection,
getVariantSearchManager().getSolrClient(),
getVariantSearchManager().getSolrManager().newSolrClient(configuration.getSearch().getWriteTimeout()), true,
getVariantSearchManager().getInsertBatchSize());
}

Expand Down
19 changes: 19 additions & 0 deletions ...java/org/opencb/opencga/storage/core/variant/search/solr/SolrInputDocumentDataWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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();
Expand Down
5 changes: 0 additions & 5 deletions ...c/main/java/org/opencb/opencga/storage/core/variant/search/solr/VariantSearchManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,11 +693,6 @@ public SolrClient getSolrClient() {
return solrManager.getSolrClient();
}

public VariantSearchManager setSolrClient(SolrClient solrClient) {
this.solrManager.setSolrClient(solrClient);
return this;
}

public SolrQueryParser getSolrQueryParser() {
return solrQueryParser;
}
Expand Down
3 changes: 2 additions & 1 deletion opencga-storage/opencga-storage-core/src/main/resources/storage-configuration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ search:
- ${OPENCGA.STORAGE.SEARCH.HOST}
configSet: "${OPENCGA.STORAGE.SEARCH.CONFIG_SET}"
mode: "cloud"
timeout: ${OPENCGA.STORAGE.SEARCH.TIMEOUT}
timeout: ${OPENCGA.STORAGE.SEARCH.TIMEOUT} # Timeout (in milliseconds) for read operations
writeTimeout: ${OPENCGA.STORAGE.SEARCH.WRITE.TIMEOUT} # Timeout (in milliseconds) for write operations
insertBatchSize: 5000

## Clinical database for indexing the pathogenic variants reported.
Expand Down
3 changes: 2 additions & 1 deletion .../test/java/org/opencb/opencga/storage/core/variant/VariantStorageSearchIntersectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.opencb.commons.datastore.core.ObjectMap;
import org.opencb.commons.datastore.core.Query;
import org.opencb.commons.datastore.core.QueryOptions;
import org.opencb.commons.datastore.solr.SolrManager;
import org.opencb.opencga.core.api.ParamConstants;
import org.opencb.opencga.storage.core.variant.query.VariantQueryResult;
import org.opencb.opencga.storage.core.StoragePipelineResult;
Expand Down Expand Up @@ -92,8 +93,8 @@ public void before() throws Exception {
// new Exception().printStackTrace();
// return invocation.callRealMethod();
// }).when(solrClient).query(anyString(), any());
// Configure variant storage engine by using the current solrClient
solr.configure(this.variantStorageEngine);
variantStorageEngine.getVariantSearchManager().setSolrClient(solrClient);
for (VariantQueryExecutor variantQueryExecutor : variantStorageEngine.getVariantQueryExecutors()) {
if (variantQueryExecutor instanceof SearchIndexVariantQueryExecutor) {
this.variantQueryExecutor = ((SearchIndexVariantQueryExecutor) variantQueryExecutor);
Expand Down
5 changes: 2 additions & 3 deletions ...c/test/java/org/opencb/opencga/storage/core/variant/solr/VariantSolrExternalResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,8 @@ 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.setSolrClient(solrClient);
// Configure variant search manger by using the current solrClient
variantSearchManager.setSolrManager(new SolrManager(solrClient, "localhost", "core"));
return variantSearchManager;
}

Expand Down
1 change: 1 addition & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down