fix(core): Update indexer controller to avoid TypeORM memory leak

[carathorys]

Description

Previously I've tried to fix the search index plugin in order to avoid a memory leak I've tought was caused because of multiple channels. In order to fix it, I've changed the default indexer controller to use queryBuilder to fetch data from the database.
Since that, I've discovered a bug in TypeORM (at least I think it's related to TypeORM), which causes the memory leak issue, and also discovered that the memory leak still exists in the indexer controller when I do a partial search index update, initiated from the admin-ui.

To avoid that, I've updated the indexer controller once again to use TypeORM repositories (instead of query builder), but instead of simply join tables, we have to explicitly say that the relationLoadStrategy should be query instead of join (which is the default behavior I think). This works properly as far as I can see, otherwise, if you would join multiple tables, TypeORM will eat all your memory instead of returning with the data you've requested.

PS.: I think this TypeORM issue was introduced with TypeORM 0.3.20, as I didn't saw this issue in earlier versions of Vendure either, only since 2.2.x

Breaking changes

Everything should work as before.

Screenshots

You can add screenshots here if applicable.

Checklist

📌 Always:

• I have set a clear title
• My PR is small and contains a single feature
• I have checked my own PR

👍 Most of the time:

• I have added or updated test cases
• I have updated the README if needed

[netlify]

❌ Deploy Preview for effervescent-donut-4977b2 failed.

Name	Link
🔨 Latest commit	d7db840
🔍 Latest deploy log	https://app.netlify.com/sites/effervescent-donut-4977b2/deploys/6666e441aea7780009570509

[tianyingchun]

it seems that e2e failed.

[michaelbromley]

Are you able to figure out why the e2e tests are failing?

[tianyingchun]

i am sorry, i am no much familiar typeorm(DB). maybe need. @carathorys help

[carathorys]

Hi!
When I raised the PR, I've checked the E2E tests, and they were passed on local machine using the same db versions.
I've seen that some E2E tests (from the file default-search-plugin-sort-by.e2e-spec.ts) were failing, but I've ignored them as I thought they were not ready yet.

Anyway, I planned to take a look on it, and fix if there are any issues, but I can't do it before the end of the next week.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	Jul 8, 2024 7:19am

[carathorys]

@michaelbromley
I've tried to check these tests today, and on local everything seems to be fine, I don't really understand why it is failing in GH actions.
What I saw is if I pick a single test, and execute it, sometimes it fails as I think these tests should be executed in order, as some of them will depend on the previous test.

Can you take a look on it please? I'm out of ideas now.

[tianyingchun]

i have tried to copy your code to my local, above test case also failed, revert these code all e2e cases works fine

all cases in a test. file will run one by one

[carathorys]

@tianyingchun Did you tried to execute all the DefaultSearchPlugin tests, or just some of them?
If I try to run all of them, it succeeds (both Shop API, and Admin API)
If I try only some of them, it randomly fails

[tianyingchun]

i have redefined- default search plugin because i have some customized fifelds. but i have copy all test cases relatated default search it works fine on vendure 2.2.6

i try update you PR into my local default search plugin some e2e case failed.

[tianyingchun]

[carathorys]

Ok, hopefully I've found the issue:
I thought that the tests would be executed on the source code, but as it turned out it will be executed on the compiled code.
Anyways, I've reverted the bigger change, and only updated the necessary part to use the relationLoadStrategy: 'query' as it solves the problem what I've experienced with the partial updates.
Also, now the tests are passing hopefully :)

[tianyingchun]

Unit tests are supposed to run source code, but not in your local environment?

[carathorys]

These are e2e tests, not unit tests, and in order to execute them on your code, first you need to build the packages :)
I've had a similar issue previously as well, I've just forgot about it.

[tianyingchun]

it seems that search builder will lose productVariantPreview & productVariantAssetId, and the e2e missed a serias cases for productVariantAssets

[tianyingchun]

After `reIndex` we will missed above fields, maybe we need to add more e2e for `searchResult`

[carathorys]

The product.featuredAsset was there, only the productVariant.featuredAsset was missing, and not from the GraphQL definition, but only from the database.
I double checked by generating the graphql definitions on my project, and it's there, and available for me. How did you checked it?

[tianyingchun]

i have not migration above code to my project now,:), just found there missed variant featured assets :)

[tianyingchun]

and this PR have give us more performance improved?

[carathorys]

Actually, we have 4 channels with 4 different languages, and I've discovered some kind of strange memory issue earlier.
Back then I made a fix (to use QueryBuilder), and I thought that everything is all right, but suddenly I've realized that the memory issue still persists when you do partial updates. The strange thing was that it happend instantaneously, and the relationLoadStrategy doesn't really works when you use queryBuilder.
So I've reverted basically the QueryBuilder change, and now it seems to be working properly, at least we don't have memory issues since I've applied this to the @vendure/core package.

EDIT:
Also, I've tried to optimize what we're loading, and I've also added the relationLoadStrategy: 'query' for the DB statements, now it seems like it solved the memory issue.

PS:
The memory issue is not in Vendure, but in typeorm 0.3.20 I think.

[tianyingchun]

nice, try it today :)
it seems that variantRelations

it seems that we do not need to load below relations for variant

  'product',
  'product.translations',
  'product.channels',
  'product.facetValues',
  'product.facetValues.facet',

[tianyingchun]

and it seems that getAllChannels can not works correctly in mysql

  const affectedChannels = await this.getAllChannels(ctx, {
        where: {
          availableLanguageCodes: In(
            product.translations.map((t) => t.languageCode)
          ),
        },
      });

    @Column({ type: 'simple-array', nullable: true })
    availableLanguageCodes: LanguageCode[];

in mysql it will store as string (en,en_GB,de) In always return empty

typeorm/typeorm#4853

[michaelbromley]

Thanks for your work on this. I just tested locally and it works well. It also resolves #2907.