fix(elasticsearch-plugin): load indexing relations with the query strategy - #43
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
…ategy The two find() calls in updateProductsOperationsOnly load several to-many relations at once. With TypeORM's default 'join' strategy these are resolved in a single query, so the result set is the cartesian product of every to-many relation, and the row count grows multiplicatively with the number of variants, facet values, collections, channels and translations. Switching to the 'query' strategy resolves each relation with its own statement, keeping the row count linear. Relates to vendurehq/vendure#4480
8f4520b to
df5174e
Compare
|
Correcting my earlier comment: both objections I raised were wrong. The numbers in the description hold up. I claimed the 810k row figure did not reproduce because I measured 3,200 rows. Different catalogue, and I misread yours. Yours is 200 variants x 10 facet values x 15 collections x 3 channels x 3 languages x prices, which lands at ~810,000. Mine was 4 facet values, 4 collections, 1 channel, 1 language, so 200 x 4 x 4 = 3,200. I also left translations and prices out of my row count entirely, which is where two of your multipliers come from. Both numbers are correct for their own shape. I also misread the crossover claim. "From roughly 20 variants upwards the difference becomes orders of magnitude" is not a claim about where My independent run, for what it is worth as corroboration. SQL layer only, Postgres 16, median of 10 runs, 4 facet values / 4 collections / 1 channel / 1 language per variant:
Same shape as yours, smaller magnitude because the catalogue is thinner. The worst case I could produce is 0.93x, a 0.3ms difference inside the noise, so there is no catalogue shape where this loses meaningfully. Nothing blocking. Two optional things: A comment above the // These relations are to-many, so the default 'join' strategy returns their
// cartesian product: N variants x facet values x collections x channels rows
// for a single product. One query per relation keeps the row count linear.And "that method is the body of the full-reindex loop" undersells it. Approving. |
There was a problem hiding this comment.
Requesting changes, retracting my earlier approve. CI red is caused by this change. Reproduced locally: main 99/99 pass, this branch 69 fail.
relationLoadStrategy: 'query' is fine alone. It breaks when the same find() also passes order or select and a relation lives inside an embedded entity. Minimal repro, no Vendure:
join + order -> OK
query + order -> THROW TypeError: Cannot read properties of undefined (reading 'tags')
query + select -> THROW TypeError: Cannot read properties of undefined (reading 'tags')
query (neither) -> OK
Filed upstream with that repro: typeorm/typeorm#12788 (same cause as #10821, closed as not planned, so don't wait on it).
Worse than a red check: the catch swallows the throw, so the indexer writes a synthetic placeholder document for every product. Any store hydrating a customFields.* relation silently ends up with an index of empty documents.
The perf work is sound and I want it merged, and the e2e tests are correct as they are. Fix is inline.
|
Thanks for catching this. I removed the query-level ordering and sort the loaded variants by id in memory instead. The code now explains the TypeORM edge case, and a variant-loading failure is rethrown so it cannot silently replace real documents with synthetic ones. |
Relates to vendurehq/vendure#4480
Summary
ElasticsearchIndexerController.updateProductsOperationsOnlyloads its relations with TypeORM's defaultjoinstrategy, which resolves every relation in a single query. Because most of the configured relations are to-many, the result set is their cartesian product.find()calls torelationLoadStrategy: 'query'makes each relation its own statement, keeping the row count linear.Changes
packages/elasticsearch-plugin/src/indexing/indexer.controller.tsrelationLoadStrategy: 'query'to theProductandProductVariantqueries inupdateProductsOperationsOnlyWhy the row count explodes
The relations loaded for each product are:
facetValues,collectionsandchannelsare many-to-many, andtranslationsandproductVariantPricesare eager one-to-many, so the joined result for the variant query is roughly:Benchmarks
Measured locally against PostgreSQL 17 and a real Elasticsearch 9.3.4 instance, with a synthetic catalogue seeded at several shapes.
End-to-end full reindex (190 products, ~3,400 variants)
join(current)query(this PR)Roughly 7–8x faster, and the resulting index is identical.
Isolated — the two
find()calls only, median per productThe trade-off is visible in the first row: for a trivial catalogue the
querystrategy costs ~3 ms more per product, because it issues 21 statements wherejoinissues 2. From roughly 20 variants upwards the join's cartesian product dominates and the difference becomes orders of magnitude.Correctness
Both strategies were compared field by field on every shape — variant ids, facet value ids, facet ids, collection ids, channel ids, tax zone ids, translations and prices all match, and the end-to-end runs produced the same 24,568 Elasticsearch documents.
Scope
Only these two queries are affected. The other database access in the plugin is either a
createQueryBuilder(whererelationLoadStrategyhas no effect) or a single many-to-one relation (getProductIdsByVariantIds), where the defaultjoinstrategy is already the better choice.Test plan
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.