Skip to content

Commit

Permalink
perf(Accounts): optimize by having a pre-check on ElasticSearch
Browse files Browse the repository at this point in the history
  • Loading branch information
Betree committed Dec 31, 2024
1 parent 066d61a commit 3d01828
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 15 deletions.
2 changes: 1 addition & 1 deletion scripts/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ program
}

const indexInputs = indexes.map(index => ({ index }));
const result = await elasticSearchGlobalSearch(indexInputs, query, { account, host, limit, user });
const result = await elasticSearchGlobalSearch(query, indexInputs, { account, host, limit, user });
console.log('Result', JSON.stringify(result, null, 2));
});

Expand Down
3 changes: 1 addition & 2 deletions server/graphql/loaders/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ type SearchParams = {
indexParams: ElasticSearchIndexParams[ElasticSearchIndexName];
forbidPrivate?: boolean;
limit: number;
adminOfAccountIds: number[];
account: Collective;
host: Collective;
};
Expand Down Expand Up @@ -86,7 +85,7 @@ export const generateSearchLoaders = req => {
const firstRequest = groupedRequests[requestId][0];
const { searchTerm, limit, account, host } = firstRequest;
const indexes = getSearchIndexes(groupedRequests[requestId]);
const results = await elasticSearchGlobalSearch(indexes, searchTerm, {
const results = await elasticSearchGlobalSearch(searchTerm, indexes, {
user: req.remoteUser,
account,
host,
Expand Down
41 changes: 41 additions & 0 deletions server/graphql/v2/query/collection/AccountsCollectionQuery.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { GraphQLBoolean, GraphQLList, GraphQLNonNull, GraphQLString } from 'graphql';
import { isNil, omitBy } from 'lodash';

import { isElasticSearchConfigured } from '../../../../lib/elastic-search/client';
import { formatIndexNameForElasticSearch } from '../../../../lib/elastic-search/common';

Check failure on line 5 in server/graphql/v2/query/collection/AccountsCollectionQuery.ts

View workflow job for this annotation

GitHub Actions / lint

'formatIndexNameForElasticSearch' is defined but never used

Check failure on line 5 in server/graphql/v2/query/collection/AccountsCollectionQuery.ts

View workflow job for this annotation

GitHub Actions / typescript

'formatIndexNameForElasticSearch' is declared but its value is never read.
import { ElasticSearchIndexName } from '../../../../lib/elastic-search/constants';
import { elasticSearchGlobalSearch } from '../../../../lib/elastic-search/search';
import { searchCollectivesInDB } from '../../../../lib/sql-search';
import { GraphQLAccountCollection } from '../../collection/AccountCollection';
import { AccountTypeToModelMapping, GraphQLAccountType, GraphQLCountryISO } from '../../enum';
Expand Down Expand Up @@ -102,7 +107,43 @@ const AccountsCollectionQuery = {
? await fetchAccountWithReference(args.includeVendorsForHost).then(({ id }) => id)
: undefined;

let preFilteredCollectiveIds: number[];
if (
isElasticSearchConfigured() &&
args.searchTerm &&
args.tagSearchOperator === 'AND' &&
!args.includeArchived && // Archived collectives are not indexed in ElasticSearch
args.skipGuests && // Guests are not indexed in ElasticSearch
isNil(args.includeVendorsForHost)
) {
// TODO hostCollectiveIds
// TODO rate limiting
const elasticSearchResult = await elasticSearchGlobalSearch(
cleanTerm,
{
index: ElasticSearchIndexName.COLLECTIVES,
indexParams: omitBy(
{
types: args.type?.length ? args.type.map(value => AccountTypeToModelMapping[value]) : null,
isHost: args.isHost,
tags: args.tag,
isActive: args.onlyActive ? true : null,
},
isNil,
),
},
{
skipHighlight: true,
},
);

const bucketResult = elasticSearchResult.aggregations.by_index['buckets'][0];
const hits = bucketResult.top_hits_by_index.hits.hits;
preFilteredCollectiveIds = hits.map(hit => hit._source.id);
}

const extraParameters = {
ids: preFilteredCollectiveIds,
orderBy: args.orderBy || { field: 'RANK', direction: 'DESC' },
types: args.type?.length ? args.type.map(value => AccountTypeToModelMapping[value]) : null,
hostCollectiveIds,
Expand Down
1 change: 1 addition & 0 deletions server/lib/elastic-search/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ export interface ElasticSearchIndexParams extends Record<ElasticSearchIndexName,
type?: CollectiveType;
isHost?: boolean;
tags?: string[];
isActive?: boolean;
};
}
30 changes: 18 additions & 12 deletions server/lib/elastic-search/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const getIndexConditions = (
case ElasticSearchIndexName.COLLECTIVES:
params = params as ElasticSearchIndexParams[ElasticSearchIndexName.COLLECTIVES];
return [
...(typeof params.isActive === 'boolean' ? [{ term: { isActive: params.isActive } }] : []),
...(params.type ? [{ term: { type: params.type } }] : []),
...(!isNil(params.isHost) ? [{ term: { isHostAccount: params.isHost } }] : []),
...(!isNil(params.tags) && !isEmpty(params.tags) ? [{ terms: { tags: params.tags } }] : []),
Expand Down Expand Up @@ -184,24 +185,27 @@ const buildQuery = (
};

export const elasticSearchGlobalSearch = async (
requestedIndexes: ElasticSearchIndexRequest[],
searchTerm: string,
requestedIndexes: ElasticSearchIndexRequest | Array<ElasticSearchIndexRequest>,
{
account,
host,
timeoutInSeconds = 30,
limit = 50,
user,
skipHighlight = false,
}: {
account?: Collective;
host?: Collective;
timeoutInSeconds?: number;
limit?: number;
user?: User;
skipHighlight?: boolean;
} = {},
) => {
const client = getElasticSearchClient({ throwIfUnavailable: true });
const { query, searchedFields, indexes } = buildQuery(searchTerm, requestedIndexes, user, account, host);
const indexesInputArray = Array.isArray(requestedIndexes) ? requestedIndexes : [requestedIndexes];
const { query, searchedFields, indexes } = buildQuery(searchTerm, indexesInputArray, user, account, host);

// Due to permissions, we may end up searching on no index at all (e.g. trying to search for comments while unauthenticated)
if (indexes.size === 0) {
Expand Down Expand Up @@ -232,16 +236,18 @@ export const elasticSearchGlobalSearch = async (
// We only need to retrieve the IDs, the rest will be fetched by the loaders
includes: ['id', 'uuid'],
},
highlight: {
pre_tags: ['<mark>'],
post_tags: ['</mark>'],
fragment_size: 40,
number_of_fragments: 1,
fields: Array.from(searchedFields).reduce((acc, field) => {
acc[field] = {};
return acc;
}, {}),
},
highlight: skipHighlight
? undefined
: {
pre_tags: ['<mark>'],
post_tags: ['</mark>'],
fragment_size: 40,
number_of_fragments: 1,
fields: Array.from(searchedFields).reduce((acc, field) => {
acc[field] = {};
return acc;
}, {}),
},
},
},
},
Expand Down
4 changes: 4 additions & 0 deletions server/lib/sql-search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ export const searchCollectivesInDB = async (
offset = 0,
limit = 100,
{
ids,
countries,
hasCustomContributionsEnabled,
hostCollectiveIds,
Expand All @@ -226,6 +227,7 @@ export const searchCollectivesInDB = async (
types,
...args
}: {
ids?: number[];
countries?: string[];
currency?: string;
hasCustomContributionsEnabled?: boolean;
Expand Down Expand Up @@ -342,6 +344,7 @@ export const searchCollectivesInDB = async (
AND c.name != 'incognito'
AND c.name != 'anonymous'
AND c."isIncognito" = FALSE ${dynamicConditions}
${!isEmpty(ids) ? `AND c.id IN (:ids)` : ''}
${!isEmpty(args.consolidatedBalance) ? `AND ${CONSOLIDATED_BALANCE_SUBQUERY} ${getAmountRangeQuery(args.consolidatedBalance)}` : ''}
ORDER BY __sort__ ${orderBy?.direction || 'DESC'}
OFFSET :offset
Expand All @@ -351,6 +354,7 @@ export const searchCollectivesInDB = async (
model: models.Collective,
mapToModel: true,
replacements: {
ids,
types,
term: term,
slugifiedTerm: term ? slugify(term) : '',
Expand Down

0 comments on commit 3d01828

Please sign in to comment.