Performance: bulk record upload is N+1 on sessions and returns every record in the group

[gdevenyi]

Current Implementation and Limitations

InstrumentRecordsService.upload() (apps/api/src/instrument-records/instrument-records.service.ts:352) handles bulk record upload — the endpoint whose entire purpose is to ingest many records at once. It currently performs a per-record chain of database calls and then returns far more data than it created.

Per-record session creation (:388-425). Every uploaded record gets its own sessionsService.create(...) call inside a Promise.all(records.map(...)). Each SessionsService.create (sessions.service.ts:31) is itself 3–5 round trips:

• resolveSubject → subjectsService.findById (and create on miss)
• prismaClient.user.findFirst({ where: { username } }) — the same username, re-queried once per record
• possibly groupsService.findById + subjectsService.addGroupForSubject
• sessionModel.create
• sessionModel.findUnique to re-read the row that was just created

So uploading N records issues roughly 4N–5N queries. The Promise.all makes them concurrent rather than sequential, which helps latency but means N × 5 queries are fired at the connection pool simultaneously — with a large upload this saturates the pool and starves every other in-flight request.

Redundant user lookup. username is a single value for the whole upload and is already validated once at :368 (usersService.findByUsername). Looking it up again inside every SessionsService.create is pure waste.

The return value is wrong-sized (:430-435). After createMany, the method returns:

return this.instrumentRecordModel.findMany({
  where: { groupId, instrumentId }
});

This is every record for that group and instrument — not the records that were just uploaded. Upload 10 records into a group that already holds 25,000 for that instrument and the response contains all 25,010, full data and computedMeasures included. The response grows with the size of the database rather than the size of the request, and the caller (useUploadInstrumentRecordsMutation) has no use for the pre-existing rows.

Error handling amplifies it. The catch at :436 calls sessionsService.deleteByIds(...), which is fine, but the failure path is reached only after all N sessions have been created — so a validation failure on the last record still costs the full N × 5 queries plus the cleanup.

Associated Application Components

Server

Proposed Solution

1. Hoist the invariants out of the loop. username → user, groupId → group, and the subject set are all resolved once for the whole upload. subjectsService.createMany is already called at :386 for exactly this reason; the session path should follow the same pattern.

2. Validate before writing anything. Run the instrument.validationSchema.safeParse pass over all records first, and only start creating sessions once every record is known to be valid. This removes the create-then-roll-back path in the common failure case.

3. Batch the session creation. Build the session rows in memory and insert them with a single createMany, then map record → sessionId locally:

const sessionRows = records.map((record) => ({
  date: record.date,
  groupId: groupId ?? null,
  subjectId: record.subjectId,
  type: 'RETROSPECTIVE' as const,
  userId: user?.id ?? null
}));
await this.sessionModel.createMany({ data: sessionRows });

Since MongoDB createMany does not return the inserted ids, either pre-generate the ObjectIds application-side (as AssignmentsService.create already does with crypto.randomUUID()) or re-query once with a single findMany. Either way this is O(1) round trips instead of O(N).

4. Return only what was created. Replace the findMany({ groupId, instrumentId }) with the ids that were just inserted — or, since the client does not appear to need the bodies, just a count. This is the single highest-impact change in the function and it is small.

5. Wrap the whole thing in a transaction. The current manual compensation (deleteByIds in the catch) is best-effort and leaves orphaned subjects behind if it fails. prismaClient.$transaction is already used in SubjectsService.deleteById, and the replica set is configured for it (MONGO_REPLICA_SET=rs0 in docker-compose.yaml).

Estimated Difficulty

Medium

Priority

Medium

[joshunrau]

@gdevenyi Claude can do this

[gdevenyi]

#1424 covers the response scoping (item 4), up-front validation (item 2) and removes a redundant re-read in SessionsService.create. Items 1, 3 and 5 are still open — sessions are still created one per record.

Worth recording why I didn't batch them there. Doing so means reproducing SessionsService.create's subject find-or-create and group association inline in upload, and that method has a quirk that should be settled first:

let group: Group | null = null;
if (groupId && !subject.groupIds.includes(groupId)) { group = await this.groupsService.findById(groupId); … }
// …
group: group ? { connect: { id: group.id } } : undefined,

group is only non-null when the subject was not already in the group, so a session created for an existing group member is persisted with groupId: null. That appears to be a bug in its own right, and it explains why uploaded records show groupId: null. Batching properly probably means adding a real createMany to SessionsService and deciding that behaviour deliberately, rather than duplicating the current logic into the upload path.