Skip to content

Instrument queries read every bundle from MongoDB even when the evaluated instance is cached #1537

Description

@gdevenyi

Current Implementation and Limitations

InstrumentsService.find loads full instrument documents, including the bundle source, for every instrument the user can read (instruments.service.ts#L285). It then calls getInstrumentInstance, which returns the cached evaluated instance when one exists and ignores the bundle (#L470-L483). After the first request, most bundles that find reads are not used. The cache is a Map with no size limit and no eviction.

find runs on:

  • each GET /v1/instruments/info call (#L378). It runs a second time for kind=SERIES (#L384-L385), but apps/web does not send that filter.
  • each GET /v1/instruments/list call (#L491).
  • each dashboard load, because count() is find().length (#L82-L87) and SummaryService calls it (summary.service.ts#L39). The dashboard also calls /v1/instruments/info (dashboard.tsx#L28), so one dashboard load runs find twice.

The cost scales with the total size of all bundles, not with the number of instruments the request needs. The built-in bundles are about 44 KB in total, but a bundle that inlines media as data URLs can be many megabytes. A projection saves the transfer from MongoDB and the BSON decode. MongoDB still reads each full document from storage.

To measure the size of the bundles on a deployment:

db.InstrumentModel.aggregate([
  { $project: { s: { $bsonSize: "$$ROOT" } } },
  { $group: { _id: null, n: { $sum: 1 }, totalMB: { $sum: { $divide: ["$s", 1048576] } }, maxKB: { $max: { $divide: ["$s", 1024] } } } }
])

Also found while checking this code (fix here or in separate issues):

  • SummaryService calls count() with no ability and no group IDs. Thus the dashboard instrument count includes series instruments that belong to other groups.
  • A cold cache does not merge concurrent loads. Two requests that need the same uncached bundle both evaluate it.
  • find can race with deleteById. If find reads a row before deleteById removes it, getInstrumentInstance can put the deleted instance back into the cache after deleteById clears it (#L262-L266).

Associated Application Components

Server

Proposed Solution

  1. Move the where clause of find into a private helper, so that find and count use the same filter.

  2. In find, select only id with that where clause. Then read bundle only for the IDs that are not in virtualizationService.context.instruments:

    const rows = await this.instrumentModel.findMany({ select: { id: true }, where });
    const cache = this.virtualizationService.context.instruments;
    const missingIds = rows.map((row) => row.id).filter((id) => !cache.has(id));
    if (missingIds.length > 0) {
      const uncached = await this.instrumentModel.findMany({
        select: { bundle: true, id: true },
        where: { id: { in: missingIds } }
      });
      await this.instantiate(uncached);
    }
    const instances = rows.flatMap((row) => cache.get(row.id) ?? []);

    Do not write cache.get(row.id)!. A concurrent deleteById can remove a row between the two queries, and the ! then hides an undefined that fails later. Keep the kind filter after instantiation, as now (#L310-L314).

  3. When count() has no kind filter, use instrumentModel.count({ where }). The dashboard calls it without arguments. This changes one behavior: now, one bundle that fails to evaluate makes /v1/summary return 500 (#L472-L478). After the change, it does not. State this in the PR.

Tests.

  • Unit tests: find does not select bundle for cached IDs; find drops a row whose instance is not in the cache; count() without kind calls instrumentModel.count and does not evaluate bundles.
  • The existing unit test at instruments.service.spec.ts#L511 asserts the exact findMany arguments. It must change with the new query shape.
  • e2e: the existing e2e tests for the instrument list and the dashboard do not assert the instrument count. Add an assertion on the dashboard instrument count, so that a wrong where in count() fails.

Estimated Difficulty

Low

Priority

Low

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions