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

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

@gdevenyi

@gdevenyi Claude can do this

#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.

perf(api): scope the upload response and validate before writing

perf(api): batch session creation and scope the upload response

perf(api): scope the upload response and validate before writing

perf(api): batch session creation and scope the upload response

Partially fixed on main (d86740e). Items 1-4 landed in #1427 (24e3775, merged 2026-09-23) and #1556 (49fd5a9). The groupId quirk from the comment above is fixed too: SessionsService.createMany resolves the group whenever groupId is set and stamps it on every session (apps/api/src/sessions/sessions.service.ts:68, :88), pinned by apps/api/src/sessions/__tests__/sessions.service.test.ts:61 and testing/src/specs/sessions.spec.ts:14. Only item 5 is open.

What is left

The compensation never covered subjects, even when deleteByIds succeeds. The catch at apps/api/src/instrument-records/instrument-records.service.ts:561-564 deletes sessions only. A failed record insert keeps the subjects created at sessions.service.ts:73 and the group pushed onto existing subjects at :74-79. A rejected upload still enrols its subjects in the group.
Validation now runs first, but the catch is still reachable in two ways. A computed measure can throw: computeMeasures runs inside the try at :550. Or the database can fail, for example on a dropped connection or a record over MongoDB's 16 MiB document limit. The 50 MiB body limit lets such a record through (libnest 9.0.0 src/app/app.container.ts:31).

Plan

Use the array form of $transaction, which subjects.service.ts:146 and instrument-records.service.ts:169 already use. Session ids are generated before the insert (sessions.service.ts:83), so no write needs another write's result. The reads (user, group, existing subjects) can run before the transaction. The interactive form would need a tx client passed through three services, because libnest's injected models are bound to the root client (libnest src/modules/prisma/prisma.module.ts:15-16), and its 5 s default timeout raised.

SubjectsService: split createMany into an awaited existence read and a returned write. Drop async from addGroupForSubjects so it returns the updateMany itself. An async function that returns a PrismaPromise runs the query right away. $transaction then throws "All elements of the array need to be Prisma Client promises".
SessionsService: inject the client with @InjectPrismaClient(). Add a prepareCreateMany that returns { ids, writes }, and have createMany run $transaction([...writes, readBack]). A failed read-back query then rolls back with the writes, so the try/catch at :96-102 can go. deleteByIds is then left with no caller and no test, so delete it.
upload: call prepareCreateMany, build the record rows, then run $transaction([...writes, instrumentRecordModel.createMany(...)]) keyed on ids[index]. Delete the try/catch at :546-564. The measures are computed before the transaction starts, so a throwing measure writes nothing. Upload only uses the session ids (:557, :571), so it can skip the session read-back and its include: { subject: true }.

create() delegates to createMany, so sessions.controller.ts:20, gateway.synchronizer.ts:159 and demo.service.ts:128 also get atomic subject and session writes. The synchronizer's cleanup at gateway.synchronizer.ts:217-225 is not covered, because its try also spans the gateway call deleteRemoteAssignment (:216). That is a separate change, and the fix for #1637 touches the same method.

Gotchas

MongoDB aborts a transaction that runs longer than transactionLifetimeLimitSeconds, 60 s by default. Time an upload near the body limit against the docker-compose replica set.
Two concurrent uploads that add the same existing subject to a group both succeed today, because addGroupForSubjects filters on NOT: { groupIds: { has: groupId } } (subjects.service.ts:53). Inside transactions, MongoDB aborts one of them with a write conflict. Return that as an error. Nothing was written, so the client can safely retry.
Prisma 8's MongoDB client has no transaction API. Its supported databases page lists MongoDB transactions as "Not yet", and prisma/orm#30765 is open. This plan adds two $transaction calls to the two in apps/api, and Upgrade to Prisma 7 (blocked on libnest, esbuild-plugin-prisma and MongoDB support) #1625 would have to port all four.

Tests

Unit: in apps/api/src/sessions/__tests__/sessions.service.test.ts, replace the test at :202. The new test asserts that the subject, group and session writes and the read-back go to one $transaction in that order, like apps/api/src/subjects/__tests__/subjects.service.test.ts:296. In apps/api/src/instrument-records/__tests__/instrument-records.service.test.ts, replace :566 and :574 with one test: the prepared writes and then the record insert go to one $transaction. Add a second test that a throwing measure starts no transaction. Mocked Prisma cannot show a rollback.
E2E: add 'should write nothing when the record insert fails' to testing/src/specs/upload.spec.ts. Upload to a new group with two records. One is for a new subject. The other is for a subject made with api.createSubject(otherGroup.id) and carries a reasonNotSatisfied over 16 MiB (the field is an unbounded z.string()). Expect a non-2xx response. Expect a 404 from GET /api/v1/subjects/<new id>, as the FILE instrument test at :89 does. Expect the existing subject's groupIds to still be [otherGroup.id]. Confirm the test fails on main first.

#1660 rewords the comment at instrument-records.service.ts:568, and #1825 appends a describe block to upload.spec.ts. Both are trivial rebases.

_{Triage note written with Claude Code. Code references point at main @ d86740e.}

Navigation Menu

Repository navigation

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

Current Implementation and Limitations

Associated Application Components

Proposed Solution

Estimated Difficulty

Priority

What is left

Plan

Gotchas

Tests

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Current Implementation and Limitations

Associated Application Components

Proposed Solution

Estimated Difficulty

Priority

Activity

joshunrau commented on Jul 20, 2026

gdevenyi commented on Jul 20, 2026

joshunrau commented on Oct 26, 2026

What is left

Plan

Gotchas

Tests

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions