Skip to content

Commit

Permalink
Index Set Page Performance Improvements (#15155)
Browse files Browse the repository at this point in the history
* And new paginated & search endpoint

* Get IndexSet from config

* improve existing pagination & calculate only relevant index set stats

* pagination response

* add changelog

* fix limit and compare without case

* Migrate IndexSetsComponent to typescript

* add pagination to search endpoint

* add test

* remove unused

* Add toggle for stats and search bar

* Fix pagination updates, remove pagination from component state

* Add styles for toolbar

* Also reset page after search reset

* fix indention

* Align search and toolbar below each other and remove search label

* Fix linter errors

---------

Co-authored-by: Maxwell Anipah <[email protected]>
Co-authored-by: Maxwell <[email protected]>
  • Loading branch information
3 people authored May 4, 2023
1 parent ca386fc commit e35059a
Show file tree
Hide file tree
Showing 12 changed files with 485 additions and 338 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/issue-12498.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type = "fixed"
message = "We improved the index set overview page performance and added a title-based search feature."

issues = ["12498"]
pulls = ["15155"]
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.graylog2.indexer;

import org.graylog2.indexer.indexset.IndexSetConfig;
import org.graylog2.indexer.indices.TooManyAliasesException;

import java.util.Collection;
Expand Down Expand Up @@ -55,6 +56,14 @@ public interface IndexSetRegistry extends Iterable<IndexSet> {
*/
Set<IndexSet> getForIndices(Collection<String> indices);

/**
* Returns the {@link IndexSet}s for the given indices.
*
* @param indexSetConfigs Collection of index configurations
* @return Set of index sets which relates to given configurations
*/
Set<IndexSet> getFromIndexConfig(Collection<IndexSetConfig> indexSetConfigs);

/**
* Returns the {@link IndexSet} that is marked as default.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ private Set<MongoIndexSet> findAllMongoIndexSets() {
}
return mongoIndexSets.build();
}

@Override
public Set<IndexSet> getAll() {
return ImmutableSet.copyOf(findAllMongoIndexSets());
Expand All @@ -110,10 +109,10 @@ public Set<IndexSet> getAll() {
@Override
public Optional<IndexSet> get(final String indexSetId) {
return this.indexSetsCache.get()
.stream()
.filter(indexSet -> Objects.equals(indexSet.id(), indexSetId))
.map(indexSetConfig -> (IndexSet)mongoIndexSetFactory.create(indexSetConfig))
.findFirst();
.stream()
.filter(indexSet -> Objects.equals(indexSet.id(), indexSetId))
.map(indexSetConfig -> (IndexSet) mongoIndexSetFactory.create(indexSetConfig))
.findFirst();
}

@Override
Expand All @@ -140,6 +139,16 @@ public Set<IndexSet> getForIndices(Collection<String> indices) {
return resultBuilder.build();
}

@Override
public Set<IndexSet> getFromIndexConfig(Collection<IndexSetConfig> indexSetConfigs) {
final ImmutableSet.Builder<MongoIndexSet> mongoIndexSets = ImmutableSet.builder();
for (IndexSetConfig config : indexSetConfigs) {
final MongoIndexSet mongoIndexSet = mongoIndexSetFactory.create(config);
mongoIndexSets.add(mongoIndexSet);
}
return ImmutableSet.copyOf(mongoIndexSets.build());
}

@Override
public IndexSet getDefault() {
return mongoIndexSetFactory.create(indexSetService.getDefault());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,14 @@ public interface IndexSetService {
* Retrieve a paginated set of index set.
*
* @param indexSetIds List of inde set ids to return
* @param limit Maximum number of index sets
* @param skip Number of index sets to skip
* @param limit Maximum number of index sets
* @param skip Number of index sets to skip
* @return Paginated index sets
*/
List<IndexSetConfig> findPaginated(Set<String> indexSetIds, int limit, int skip);

List<IndexSetConfig> searchByTitle(String searchString);

/**
* Save the given index set.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
import javax.inject.Inject;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static com.google.common.base.Preconditions.checkState;
Expand Down Expand Up @@ -153,6 +155,16 @@ public List<IndexSetConfig> findPaginated(Set<String> indexSetIds, int limit, in
.toArray());
}

@Override
public List<IndexSetConfig> searchByTitle(String searchString) {
String formatedSearchString = String.format(Locale.getDefault(), ".*%s.*", searchString);
Pattern searchPattern = Pattern.compile(formatedSearchString, Pattern.CASE_INSENSITIVE);
DBQuery.Query query = DBQuery.regex("title", searchPattern);
return ImmutableList.copyOf(collection.find(query)
.sort(DBSort.asc("title"))
.toArray());
}

/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -78,7 +79,7 @@
import static org.graylog2.shared.rest.documentation.generator.Generator.CLOUD_VISIBLE;

@RequiresAuthentication
@Api(value = "System/IndexSets", description = "Index sets", tags={CLOUD_VISIBLE})
@Api(value = "System/IndexSets", description = "Index sets", tags = {CLOUD_VISIBLE})
@Path("/system/indices/index_sets")
@Produces(MediaType.APPLICATION_JSON)
public class IndexSetsResource extends RestResource {
Expand Down Expand Up @@ -124,41 +125,64 @@ public IndexSetResponse list(@ApiParam(name = "skip", value = "The number of ele
@QueryParam("limit") @DefaultValue("0") int limit,
@ApiParam(name = "stats", value = "Include index set stats.")
@QueryParam("stats") @DefaultValue("false") boolean computeStats) {

final IndexSetConfig defaultIndexSet = indexSetService.getDefault();
List<IndexSetConfig> allowedConfigurations = indexSetService.findAll()
.stream()
.filter(indexSet -> isPermitted(RestPermissions.INDEXSETS_READ, indexSet.id()))
.toList();

List<IndexSetSummary> indexSets;
int count;
return getPagedIndexSetResponse(skip, limit, computeStats, defaultIndexSet, allowedConfigurations);
}

if (limit > 0) {
// First collect all index set ids the user is allowed to see.
final Set<String> allowedIds = indexSetService.findAll().stream()
.filter(indexSet -> isPermitted(RestPermissions.INDEXSETS_READ, indexSet.id()))
.map(IndexSetConfig::id)
.collect(Collectors.toSet());
@GET
@Path("search")
@Timed
@ApiOperation(value = "Get a list of all index sets")
@ApiResponses(value = {
@ApiResponse(code = 403, message = "Unauthorized"),
})
public IndexSetResponse search(@ApiParam(name = "searchTitle", value = "The number of elements to skip (offset).")
@QueryParam("searchTitle") String searchTitle,
@ApiParam(name = "skip", value = "The number of elements to skip (offset).", required = true)
@QueryParam("skip") @DefaultValue("0") int skip,
@ApiParam(name = "limit", value = "The maximum number of elements to return.", required = true)
@QueryParam("limit") @DefaultValue("0") int limit,
@ApiParam(name = "stats", value = "Include index set stats.")
@QueryParam("stats") @DefaultValue("false") boolean computeStats) {
final IndexSetConfig defaultIndexSet = indexSetService.getDefault();
List<IndexSetConfig> allowedConfigurations = indexSetService.searchByTitle(searchTitle).stream()
.filter(indexSet -> isPermitted(RestPermissions.INDEXSETS_READ, indexSet.id())).toList();

indexSets = indexSetService.findPaginated(allowedIds, limit, skip).stream()
.map(config -> IndexSetSummary.fromIndexSetConfig(config, config.equals(defaultIndexSet)))
.collect(Collectors.toList());
count = allowedIds.size();
} else {
indexSets = indexSetService.findAll().stream()
.filter(indexSetConfig -> isPermitted(RestPermissions.INDEXSETS_READ, indexSetConfig.id()))
.map(config -> IndexSetSummary.fromIndexSetConfig(config, config.equals(defaultIndexSet)))
.collect(Collectors.toList());
count = indexSets.size();
}
return getPagedIndexSetResponse(skip, limit, computeStats, defaultIndexSet, allowedConfigurations);
}

private IndexSetResponse getPagedIndexSetResponse(int skip, int limit, boolean computeStats, IndexSetConfig defaultIndexSet, List<IndexSetConfig> allowedConfigurations) {
int calculatedLimit = limit > 0 ? limit : allowedConfigurations.size();
Comparator<IndexSetConfig> titleComparator = Comparator.comparing(IndexSetConfig::title, String.CASE_INSENSITIVE_ORDER);

List<IndexSetConfig> pagedConfigs = allowedConfigurations.stream()
.sorted(titleComparator)
.skip(skip)
.limit(calculatedLimit)
.toList();

List<IndexSetSummary> indexSets = pagedConfigs.stream()
.map(config -> IndexSetSummary.fromIndexSetConfig(config, config.equals(defaultIndexSet)))
.toList();


Map<String, IndexSetStats> stats = Collections.emptyMap();

final Map<String, IndexSetStats> stats;
if (computeStats) {
stats = indexSetRegistry.getAll().stream()
stats = indexSetRegistry.getFromIndexConfig(pagedConfigs).stream()
.collect(Collectors.toMap(indexSet -> indexSet.getConfig().id(), indexSetStatsCreator::getForIndexSet));
} else {
stats = Collections.emptyMap();
}

return IndexSetResponse.create(count, indexSets, stats);
return IndexSetResponse.create(allowedConfigurations.size(), indexSets, stats);
}


@GET
@Path("stats")
@Timed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -114,25 +115,7 @@ private void notPermitted() {

@Test
public void list() {
final IndexSetConfig indexSetConfig = IndexSetConfig.create(
"id",
"title",
"description",
true, true,
"prefix",
1,
0,
MessageCountRotationStrategy.class.getCanonicalName(),
MessageCountRotationStrategyConfig.create(1000),
NoopRetentionStrategy.class.getCanonicalName(),
NoopRetentionStrategyConfig.create(1),
ZonedDateTime.of(2016, 10, 10, 12, 0, 0, 0, ZoneOffset.UTC),
"standard",
"index-template",
null,
1,
false
);
final IndexSetConfig indexSetConfig = createTestConfig("id", "title");
when(indexSetService.findAll()).thenReturn(Collections.singletonList(indexSetConfig));

final IndexSetResponse list = indexSetsResource.list(0, 0, false);
Expand All @@ -149,25 +132,7 @@ public void list() {
public void listDenied() {
notPermitted();

final IndexSetConfig indexSetConfig = IndexSetConfig.create(
"id",
"title",
"description",
true, true,
"prefix",
1,
0,
MessageCountRotationStrategy.class.getCanonicalName(),
MessageCountRotationStrategyConfig.create(1000),
NoopRetentionStrategy.class.getCanonicalName(),
NoopRetentionStrategyConfig.create(1),
ZonedDateTime.of(2016, 10, 10, 12, 0, 0, 0, ZoneOffset.UTC),
"standard",
"index-template",
null,
1,
false
);
final IndexSetConfig indexSetConfig = createTestConfig("id", "title");
when(indexSetService.findAll()).thenReturn(Collections.singletonList(indexSetConfig));

final IndexSetResponse list = indexSetsResource.list(0, 0, false);
Expand Down Expand Up @@ -196,25 +161,7 @@ public void list0() {

@Test
public void get() {
final IndexSetConfig indexSetConfig = IndexSetConfig.create(
"id",
"title",
"description",
true, true,
"prefix",
1,
0,
MessageCountRotationStrategy.class.getCanonicalName(),
MessageCountRotationStrategyConfig.create(1000),
NoopRetentionStrategy.class.getCanonicalName(),
NoopRetentionStrategyConfig.create(1),
ZonedDateTime.of(2016, 10, 10, 12, 0, 0, 0, ZoneOffset.UTC),
"standard",
"index-template",
null,
1,
false
);
final IndexSetConfig indexSetConfig = createTestConfig("id", "title");
when(indexSetService.get("id")).thenReturn(Optional.of(indexSetConfig));

final IndexSetSummary summary = indexSetsResource.get("id");
Expand Down Expand Up @@ -408,25 +355,7 @@ public void update() {
@Test
public void updateDenied() {
notPermitted();
final IndexSetConfig indexSetConfig = IndexSetConfig.create(
"id",
"title",
"description",
true, true,
"prefix",
1,
0,
MessageCountRotationStrategy.class.getCanonicalName(),
MessageCountRotationStrategyConfig.create(1000),
NoopRetentionStrategy.class.getCanonicalName(),
NoopRetentionStrategyConfig.create(1),
ZonedDateTime.of(2016, 10, 10, 12, 0, 0, 0, ZoneOffset.UTC),
"standard",
"index-template",
null,
1,
false
);
final IndexSetConfig indexSetConfig = createTestConfig("id", "title");

expectedException.expect(ForbiddenException.class);
expectedException.expectMessage("Not authorized to access resource id <wrong-id>");
Expand Down Expand Up @@ -765,6 +694,49 @@ public void setDefaultDoesNotDoAnythingIfIndexSetIsNotElegibleAsDefault() throws
}
}

@Test
public void testSearchIndexSets() {
final IndexSetConfig indexSetConfig = createTestConfig("id", "title");
String searchTitle = "itle";
when(indexSetService.searchByTitle(searchTitle)).thenReturn(Collections.singletonList(indexSetConfig));
final IndexSetResponse firstPage = indexSetsResource.search(searchTitle, 0, 0, false);

verify(indexSetService, times(1)).searchByTitle(searchTitle);
verify(indexSetService, times(1)).getDefault();
verifyNoMoreInteractions(indexSetService);

assertThat(firstPage.total()).isEqualTo(1);
assertThat(firstPage.indexSets()).containsExactly(IndexSetSummary.fromIndexSetConfig(indexSetConfig, false));

final IndexSetConfig indexSetConfig2 = createTestConfig("id2", "title2");
when(indexSetService.searchByTitle(searchTitle)).thenReturn(List.of(indexSetConfig, indexSetConfig2));
final IndexSetResponse secondPage = indexSetsResource.search(searchTitle, 1, 0, false);

assertThat(secondPage.indexSets()).containsExactly(IndexSetSummary.fromIndexSetConfig(indexSetConfig2, false));
}

private IndexSetConfig createTestConfig(String id, String title) {
return IndexSetConfig.create(
id,
title,
"description",
true, true,
"prefix",
1,
0,
MessageCountRotationStrategy.class.getCanonicalName(),
MessageCountRotationStrategyConfig.create(1000),
NoopRetentionStrategy.class.getCanonicalName(),
NoopRetentionStrategyConfig.create(1),
ZonedDateTime.of(2016, 10, 10, 12, 0, 0, 0, ZoneOffset.UTC),
"standard",
"index-template",
null,
1,
false
);
}

private static class TestResource extends IndexSetsResource {
private final Provider<Boolean> permitted;

Expand Down
Loading

0 comments on commit e35059a

Please sign in to comment.