Skip to content

Commit

Permalink
Improved timeout and search syntax errors #267
Browse files Browse the repository at this point in the history
  • Loading branch information
patrick-austin committed Aug 5, 2022
1 parent 42bcef0 commit e7a322c
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 47 deletions.
20 changes: 12 additions & 8 deletions src/main/java/org/icatproject/core/manager/EntityBeanManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ public enum PersistMode {
private Map<String, NotificationRequest> notificationRequests;

private boolean searchActive;
private long searchMaxSearchTimeMillis;

private int maxEntities;

Expand Down Expand Up @@ -1163,6 +1164,7 @@ void init() {
log = !logRequests.isEmpty();
notificationRequests = propertyHandler.getNotificationRequests();
searchActive = searchManager.isActive();
searchMaxSearchTimeMillis = propertyHandler.getSearchMaxSearchTimeMillis();
maxEntities = propertyHandler.getMaxEntities();
exportCacheSize = propertyHandler.getImportCacheSize();
rootUserNames = propertyHandler.getRootUserNames();
Expand Down Expand Up @@ -1418,7 +1420,7 @@ public void searchCommit() throws IcatException {
*/
public List<ScoredEntityBaseBean> freeTextSearch(String userName, JsonObject jo, int limit, String sort,
EntityManager manager, String ip, Class<? extends EntityBaseBean> klass) throws IcatException {
long startMillis = log ? System.currentTimeMillis() : 0;
long startMillis = System.currentTimeMillis();
List<ScoredEntityBaseBean> results = new ArrayList<>();
JsonValue searchAfter = null;
JsonValue lastSearchAfter = null;
Expand All @@ -1433,9 +1435,6 @@ public List<ScoredEntityBaseBean> freeTextSearch(String userName, JsonObject jo,

do {
lastSearchResult = searchManager.freeTextSearch(jo, searchAfter, blockSize, sort, Arrays.asList("id"));
if (lastSearchResult.isAborted()) {
break;
}
allResults = lastSearchResult.getResults();
ScoredEntityBaseBean lastBean = filterReadAccess(results, allResults, limit, userName, manager, klass);
if (lastBean == null) {
Expand All @@ -1450,6 +1449,10 @@ public List<ScoredEntityBaseBean> freeTextSearch(String userName, JsonObject jo,
lastSearchAfter = searchManager.buildSearchAfter(lastBean, sort);
break;
}
if (System.currentTimeMillis() - startMillis > searchMaxSearchTimeMillis) {
String msg = "Search cancelled for exceeding " + searchMaxSearchTimeMillis / 1000 + " seconds";
throw new IcatException(IcatExceptionType.INTERNAL, msg);
}
} while (results.size() < limit);
}

Expand Down Expand Up @@ -1493,7 +1496,7 @@ public List<ScoredEntityBaseBean> freeTextSearch(String userName, JsonObject jo,
public SearchResult freeTextSearchDocs(String userName, JsonObject jo, JsonValue searchAfter, int minCount,
int maxCount, String sort, EntityManager manager, String ip,
Class<? extends EntityBaseBean> klass) throws IcatException {
long startMillis = log ? System.currentTimeMillis() : 0;
long startMillis = System.currentTimeMillis();
List<ScoredEntityBaseBean> results = new ArrayList<>();
JsonValue lastSearchAfter = null;
List<FacetDimension> dimensions = new ArrayList<>();
Expand All @@ -1509,9 +1512,6 @@ public SearchResult freeTextSearchDocs(String userName, JsonObject jo, JsonValue

do {
lastSearchResult = searchManager.freeTextSearch(jo, searchAfter, blockSize, sort, fields);
if (lastSearchResult.isAborted()) {
return lastSearchResult;
}
allResults = lastSearchResult.getResults();
ScoredEntityBaseBean lastBean = filterReadAccess(results, allResults, maxCount, userName, manager,
klass);
Expand All @@ -1527,6 +1527,10 @@ public SearchResult freeTextSearchDocs(String userName, JsonObject jo, JsonValue
lastSearchAfter = searchManager.buildSearchAfter(lastBean, sort);
break;
}
if (System.currentTimeMillis() - startMillis > searchMaxSearchTimeMillis) {
String msg = "Search cancelled for exceeding " + searchMaxSearchTimeMillis / 1000 + " seconds";
throw new IcatException(IcatExceptionType.INTERNAL, msg);
}
} while (results.size() < minCount);

if (jo.containsKey("facets")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ public int getLifetimeMinutes() {
private Path searchDirectory;
private long searchBacklogHandlerIntervalMillis;
private long searchAggregateFilesIntervalMillis;
private long searchMaxSearchTimeMillis;
private String unitAliasOptions;
private Map<String, String> cluster = new HashMap<>();
private long searchEnqueuedRequestIntervalMillis;
Expand Down Expand Up @@ -516,6 +517,10 @@ private void init() {

searchAggregateFilesIntervalMillis = props.getNonNegativeLong("search.aggregateFilesIntervalSeconds");
searchAggregateFilesIntervalMillis *= 1000;

searchMaxSearchTimeMillis = props.getPositiveLong("search.maxSearchTimeSeconds");
formattedProps.add("search.maxSearchTimeSeconds" + " " + searchMaxSearchTimeMillis);
searchMaxSearchTimeMillis *= 1000;
} else {
logger.info("'search.engine' entry not present so no free text search available");
}
Expand Down Expand Up @@ -670,6 +675,10 @@ public long getSearchAggregateFilesIntervalMillis() {
return searchAggregateFilesIntervalMillis;
}

public long getSearchMaxSearchTimeMillis() {
return searchMaxSearchTimeMillis;
}

public Path getSearchDirectory() {
return searchDirectory;
}
Expand Down
34 changes: 15 additions & 19 deletions src/main/java/org/icatproject/core/manager/search/LuceneApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -174,26 +174,22 @@ public SearchResult getResults(JsonObject query, JsonValue searchAfter, Integer

JsonObject postResponse = postResponse(basePath + "/" + indexPath, queryString, parameterMap);
SearchResult lsr = new SearchResult();
if (postResponse.containsKey("aborted") && postResponse.getBoolean("aborted")) {
lsr.setAborted(true);
} else {
List<ScoredEntityBaseBean> results = lsr.getResults();
List<JsonObject> resultsArray = postResponse.getJsonArray("results").getValuesAs(JsonObject.class);
for (JsonObject resultObject : resultsArray) {
int luceneDocId = resultObject.getInt("_id");
int shardIndex = resultObject.getInt("_shardIndex");
Float score = Float.NaN;
if (resultObject.keySet().contains("_score")) {
score = resultObject.getJsonNumber("_score").bigDecimalValue().floatValue();
}
JsonObject source = resultObject.getJsonObject("_source");
ScoredEntityBaseBean result = new ScoredEntityBaseBean(luceneDocId, shardIndex, score, source);
results.add(result);
logger.trace("Result id {} with score {}", result.getEntityBaseBeanId(), score);
}
if (postResponse.containsKey("search_after")) {
lsr.setSearchAfter(postResponse.getJsonObject("search_after"));
List<ScoredEntityBaseBean> results = lsr.getResults();
List<JsonObject> resultsArray = postResponse.getJsonArray("results").getValuesAs(JsonObject.class);
for (JsonObject resultObject : resultsArray) {
int luceneDocId = resultObject.getInt("_id");
int shardIndex = resultObject.getInt("_shardIndex");
Float score = Float.NaN;
if (resultObject.keySet().contains("_score")) {
score = resultObject.getJsonNumber("_score").bigDecimalValue().floatValue();
}
JsonObject source = resultObject.getJsonObject("_source");
ScoredEntityBaseBean result = new ScoredEntityBaseBean(luceneDocId, shardIndex, score, source);
results.add(result);
logger.trace("Result id {} with score {}", result.getEntityBaseBeanId(), score);
}
if (postResponse.containsKey("search_after")) {
lsr.setSearchAfter(postResponse.getJsonObject("search_after"));
}

return lsr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,8 @@ protected JsonObject postResponse(String path, String body, Map<String, String>
httpPost.setEntity(new StringEntity(body, ContentType.APPLICATION_JSON));
logger.trace("Making call {} with body {}", uri, body);
try (CloseableHttpResponse response = httpclient.execute(httpPost)) {
Rest.checkStatus(response, IcatExceptionType.INTERNAL);
int code = response.getStatusLine().getStatusCode();
Rest.checkStatus(response, code == 400 ? IcatExceptionType.BAD_PARAMETER : IcatExceptionType.INTERNAL);
JsonReader jsonReader = Json.createReader(response.getEntity().getContent());
return jsonReader.readObject();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ public class SearchResult {
private JsonValue searchAfter;
private List<ScoredEntityBaseBean> results = new ArrayList<>();
private List<FacetDimension> dimensions;
private boolean aborted;

public SearchResult() {
}
Expand Down Expand Up @@ -46,12 +45,4 @@ public void setSearchAfter(JsonValue searchAfter) {
this.searchAfter = searchAfter;
}

public boolean isAborted() {
return aborted;
}

public void setAborted(boolean aborted) {
this.aborted = aborted;
}

}
10 changes: 0 additions & 10 deletions src/main/java/org/icatproject/exposed/ICATRest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1336,11 +1336,6 @@ public String search(@Context HttpServletRequest request, @QueryParam("sessionId

JsonGenerator gen = Json.createGenerator(baos);
gen.writeStartObject();
if (result.isAborted()) {
gen.write("aborted", true).writeEnd().close();
return baos.toString();
}

JsonValue newSearchAfter = result.getSearchAfter();
if (newSearchAfter != null) {
gen.write("search_after", newSearchAfter);
Expand Down Expand Up @@ -1433,11 +1428,6 @@ public String facet(@Context HttpServletRequest request, @QueryParam("sessionId"

JsonGenerator gen = Json.createGenerator(baos);
gen.writeStartObject();
if (result.isAborted()) {
gen.write("aborted", true).writeEnd().close();
return baos.toString();
}

List<FacetDimension> dimensions = result.getDimensions();
if (dimensions != null && dimensions.size() > 0) {
gen.writeStartObject("dimensions");
Expand Down

0 comments on commit e7a322c

Please sign in to comment.