Reposting some findings from @xcrzx here:
- When a package installation is triggered, the entire archive is fetched and stored as a Buffer in external memory. This part of the code is responsible for that (see archiveBuffer).
- Then, to extract any information from the archive, it is iterated over without its content being fully extracted into memory:
- For example, here, the archive is iterated over to extract the paths of all the files it contains.
- Or here, the archive is iterated over again for validation purposes.
- And finally here, the archive is iterated once more during asset installation.
- The important part is what's happening during archive iterations. All archive entries are read one by one, then converted into Buffer, and as a result stored in external memory. This happening regardless of whether they’re needed (e.g., when extracting paths or during validation archive entries are not even accessed).
- During a normal package installation, the archive is iterated over at least three times. This means the entire archive buffer could simultaneously be stored in external memory with all archive entries (x3) if GC cycles don’t trigger between iterations. And I believe this is exactly what led to high external memory consumption in production. And since heap memory usage was below the threshold, GC was not invoked to free external memory, causing Kubernetes to kill the process.
To confirm these findings, I manually triggered a GC cycle by calling global.gc() after an iteration through archive entries. External memory consumption dropped from over 200MB to 70MB - the exact size of the package archive. That means with timely garbage collection, package installation should not consume significantly more memory than the size of the package archive.
As a fix, during archive iteration, external memory allocation should be avoided. Instead of storing entries as Buffer, they could be parsed and stored in alternative formats such as JSON, text, or Blob. Another approach would be to delay the stream-to-buffer conversion until it's actually needed. When iterating though an archive, if an entry content is not needed, its read stream can simply be closed without consuming avoiding memory allocation at all. This optimization can be applied during the archive iteration to gather paths. In this case, all archive entry streams can be closed without consuming them. And a similar approach can be applied to the archive validation step, where only a few files (e.g., the manifest) need to be read.
We should verify what our best approach to replacing usage of Buffer would be in these archive unpacking calls during package installation/upgrade, then make the changes proposed above to ensure archive data can be promptly garbage collected in order to avoid excessive external memory pressure.
Reposting some findings from @xcrzx here:
We should verify what our best approach to replacing usage of
Bufferwould be in these archive unpacking calls during package installation/upgrade, then make the changes proposed above to ensure archive data can be promptly garbage collected in order to avoid excessive external memory pressure.