From 16e55b4d2acdc9acc53c5e0b3785ade8ab6177ae Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Tue, 24 Mar 2026 21:23:39 +0100 Subject: [PATCH 1/3] fix: track total hits when doing an elastic search query to get an accurate number of total hits, cleanup --- .../openvsx/search/ElasticSearchService.java | 55 +++++++++---------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java b/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java index eda6905db..973c56369 100644 --- a/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java +++ b/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java @@ -15,6 +15,7 @@ import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; import co.elastic.clients.elasticsearch._types.query_dsl.QueryBuilders; import co.elastic.clients.util.ObjectBuilder; +import jakarta.annotation.PostConstruct; import org.apache.commons.lang3.StringUtils; import org.eclipse.openvsx.entities.Extension; import org.eclipse.openvsx.migration.HandlerJobRequest; @@ -41,7 +42,7 @@ import org.springframework.data.elasticsearch.core.query.IndexQueryBuilder; import org.springframework.retry.annotation.Retryable; import org.springframework.scheduling.annotation.Async; -import org.springframework.stereotype.Component; +import org.springframework.stereotype.Service; import org.springframework.util.StopWatch; import java.time.ZoneId; @@ -52,7 +53,7 @@ import static org.eclipse.openvsx.cache.CacheService.CACHE_AVERAGE_REVIEW_RATING; -@Component +@Service public class ElasticSearchService implements ISearchService { protected final ReadWriteLock rwLock = new ReentrantReadWriteLock(); @@ -68,7 +69,7 @@ public class ElasticSearchService implements ISearchService { @Value("${ovsx.elasticsearch.clear-on-start:false}") boolean clearOnStart; - private Long maxResultWindow; + private long maxResultWindow; public ElasticSearchService( RepositoryService repositories, @@ -81,7 +82,13 @@ public ElasticSearchService( this.relevanceService = relevanceService; this.scheduler = scheduler; } - + + @PostConstruct + public void initialize() { + var settings = searchOperations.indexOps(ExtensionSearch.class).getSettings(true); + maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString()); + } + public boolean isEnabled() { return enableSearch; } @@ -90,10 +97,10 @@ public boolean isEnabled() { * Application start listener that initializes the search index. If the application property * {@code ovsx.elasticsearch.clear-on-start} is set to {@code true}, the index is cleared * and rebuilt from scratch. If the property is {@code false} and the search index does - * not exist yet, it is created and initialized. Otherwise nothing happens. + * not exist yet, it is created and initialized. Otherwise, nothing happens. */ @EventListener - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) @CacheEvict(value = CACHE_AVERAGE_REVIEW_RATING, allEntries = true) public void initSearchIndex(ApplicationStartedEvent event) { scheduler.scheduleRecurrently("ElasticSearchUpdateIndex", Cron.daily(4), ZoneId.of("UTC"), new HandlerJobRequest<>(ElasticSearchUpdateIndexJobRequestHandler.class)); @@ -112,7 +119,7 @@ public void initSearchIndex(ApplicationStartedEvent event) { * consider the extension publishing timestamps in relation to the current * time or the extension rating. */ - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) @CacheEvict(value = CACHE_AVERAGE_REVIEW_RATING, allEntries = true) public void updateSearchIndex() { if (!isEnabled()) { @@ -134,7 +141,7 @@ public void updateSearchIndex() { * In any case, this method scans all extensions in the database and indexes their * relevant metadata. */ - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) public void updateSearchIndex(boolean clear) { var locked = false; try { @@ -180,12 +187,12 @@ public void updateSearchIndex(boolean clear) { } @Async - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) public void updateSearchEntriesAsync(List extensions) { updateSearchEntries(extensions); } - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) public void updateSearchEntries(List extensions) { if (!isEnabled() || extensions.isEmpty()) { return; @@ -205,7 +212,7 @@ public void updateSearchEntries(List extensions) { } } - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) public void updateSearchEntry(Extension extension) { if (!isEnabled()) { return; @@ -223,7 +230,7 @@ public void updateSearchEntry(Extension extension) { } } - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) public void removeSearchEntries(Collection ids) { if (!isEnabled()) { return; @@ -235,7 +242,7 @@ public void removeSearchEntries(Collection ids) { } - @Retryable(DataAccessResourceFailureException.class) + @Retryable(retryFor = DataAccessResourceFailureException.class) public void removeSearchEntry(Extension extension) { if (!isEnabled()) { return; @@ -251,7 +258,7 @@ public void removeSearchEntry(Extension extension) { public SearchResult search(Options options) { var resultWindow = options.requestedOffset() + options.requestedSize(); - if(resultWindow > getMaxResultWindow()) { + if (resultWindow > maxResultWindow) { return new SearchResult(0L, Collections.emptyList()); } @@ -263,15 +270,16 @@ public SearchResult search(Options options) { var pages = new ArrayList(); pages.add(PageRequest.of(options.requestedOffset() / options.requestedSize(), options.requestedSize())); - if(options.requestedOffset() % options.requestedSize() > 0) { + if (options.requestedOffset() % options.requestedSize() > 0) { // size is not exact multiple of offset; this means we need to get two pages // e.g. when offset is 20 and size is 50, you want results 20 to 70 which span pages 0 and 1 of a 50 item page - pages.add(pages.get(0).next()); + pages.add(pages.getFirst().next()); } var searchHitsList = new ArrayList>(pages.size()); - for(var page : pages) { + for (var page : pages) { queryBuilder.withPageable(page); + queryBuilder.withTrackTotalHits(true); try { rwLock.readLock().lock(); var searchHits = searchOperations.search(queryBuilder.build(), ExtensionSearch.class, searchOperations.indexOps(ExtensionSearch.class).getIndexCoordinates()); @@ -283,7 +291,7 @@ public SearchResult search(Options options) { var firstSearchHitsPage = searchHitsList.get(0); List> searchHits = new ArrayList<>(firstSearchHitsPage.getSearchHits()); - if(searchHitsList.size() == 2) { + if (searchHitsList.size() == 2) { var secondSearchHitsPage = searchHitsList.get(1); searchHits.addAll(secondSearchHitsPage.getSearchHits()); @@ -383,7 +391,7 @@ private void sortResults(NativeQueryBuilder queryBuilder, String sortOrder, Stri ); var type = types.get(sortBy); - if(type == null) { + if (type == null) { throw new ErrorResultException("sortBy parameter must be " + SortBy.OPTIONS + "."); } @@ -392,13 +400,4 @@ private void sortResults(NativeQueryBuilder queryBuilder, String sortOrder, Stri var sortOptions = sortBy.equals(SortBy.RELEVANCE) ? List.of(scoreSort, fieldSort) : List.of(fieldSort, scoreSort); queryBuilder.withSort(sortOptions); } - - private long getMaxResultWindow() { - if(maxResultWindow == null) { - var settings = searchOperations.indexOps(ExtensionSearch.class).getSettings(true); - maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString()); - } - - return maxResultWindow; - } } From 244080ef39df155a7198a22b54e80616731cec0b Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Tue, 24 Mar 2026 21:38:58 +0100 Subject: [PATCH 2/3] add null check due to mocks in unit tests --- .../org/eclipse/openvsx/search/ElasticSearchService.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java b/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java index 973c56369..68d68b332 100644 --- a/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java +++ b/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java @@ -85,8 +85,12 @@ public ElasticSearchService( @PostConstruct public void initialize() { - var settings = searchOperations.indexOps(ExtensionSearch.class).getSettings(true); - maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString()); + var indexOps = searchOperations.indexOps(ExtensionSearch.class); + // need to do an explicit null check as searchOperations is mocked in unit tests right now + if (indexOps != null) { + var settings = indexOps.getSettings(true); + maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString()); + } } public boolean isEnabled() { From f3f66715093520bd34c6e105efdef26f0bfbb954 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Tue, 24 Mar 2026 22:42:43 +0100 Subject: [PATCH 3/3] move maxResultWindow initialization to application event listener --- .../openvsx/search/ElasticSearchService.java | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java b/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java index 68d68b332..d071fe05d 100644 --- a/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java +++ b/server/src/main/java/org/eclipse/openvsx/search/ElasticSearchService.java @@ -15,7 +15,6 @@ import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; import co.elastic.clients.elasticsearch._types.query_dsl.QueryBuilders; import co.elastic.clients.util.ObjectBuilder; -import jakarta.annotation.PostConstruct; import org.apache.commons.lang3.StringUtils; import org.eclipse.openvsx.entities.Extension; import org.eclipse.openvsx.migration.HandlerJobRequest; @@ -83,16 +82,6 @@ public ElasticSearchService( this.scheduler = scheduler; } - @PostConstruct - public void initialize() { - var indexOps = searchOperations.indexOps(ExtensionSearch.class); - // need to do an explicit null check as searchOperations is mocked in unit tests right now - if (indexOps != null) { - var settings = indexOps.getSettings(true); - maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString()); - } - } - public boolean isEnabled() { return enableSearch; } @@ -107,15 +96,29 @@ public boolean isEnabled() { @Retryable(retryFor = DataAccessResourceFailureException.class) @CacheEvict(value = CACHE_AVERAGE_REVIEW_RATING, allEntries = true) public void initSearchIndex(ApplicationStartedEvent event) { - scheduler.scheduleRecurrently("ElasticSearchUpdateIndex", Cron.daily(4), ZoneId.of("UTC"), new HandlerJobRequest<>(ElasticSearchUpdateIndexJobRequestHandler.class)); - if (!isEnabled() || !clearOnStart && searchOperations.indexOps(ExtensionSearch.class).exists()) { + if (!isEnabled()) { + scheduler.deleteRecurringJob("ElasticSearchUpdateIndex"); return; } - var stopWatch = new StopWatch(); - stopWatch.start(); - updateSearchIndex(clearOnStart); - stopWatch.stop(); - logger.info("Initialized search index in {} ms", stopWatch.getTotalTimeMillis()); + + // schedule recurring job to update the search index + scheduler.scheduleRecurrently( + "ElasticSearchUpdateIndex", + Cron.daily(4), + ZoneId.of("UTC"), + new HandlerJobRequest<>(ElasticSearchUpdateIndexJobRequestHandler.class) + ); + + if (clearOnStart || !searchOperations.indexOps(ExtensionSearch.class).exists()) { + var stopWatch = new StopWatch(); + stopWatch.start(); + updateSearchIndex(clearOnStart); + stopWatch.stop(); + logger.info("Initialized search index in {} ms", stopWatch.getTotalTimeMillis()); + } + + var settings = searchOperations.indexOps(ExtensionSearch.class).getSettings(true); + maxResultWindow = Long.parseLong(settings.getOrDefault("index.max_result_window", "10000").toString()); } /**