Improve check performance (#763)

* Better error logs

* Upgrade to generic-filehandle2

* Add cache for seq adapters

* Add cache for refSeq documents

* Use toSorted instead of cloning and then sorting

* Fix wrong file name passed to handle

* Get sequence up front if possible when checking

Author: garrettjstevens

packages/apollo-cli/README.md

@@ -45,7 +45,7 @@
     "@emotion/react": "^11.10.6",
     "@emotion/styled": "^11.10.6",
     "@gmod/gff": "^2.0.0",
-    "@gmod/indexedfasta": "^2.0.4",
+    "@gmod/indexedfasta": "^5.0.2",
     "@jbrowse/core": "^4.1.1",
     "@jbrowse/mobx-state-tree": "^5.5.0",
     "@mui/material": "^7.3.7",
@@ -64,7 +64,7 @@
     "deepmerge": "^4.3.1",
     "express": "^4.18.0",
     "express-session": "^1.17.3",
-    "generic-filehandle": "^3.0.0",
+    "generic-filehandle2": "^2.0.18",
     "https-proxy-agent": "^7.0.6",
     "joi": "^17.7.0",
     "material-ui-popup-state": "^5.0.4",
@@ -79,6 +79,7 @@
     "passport-local": "^1.0.0",
     "passport-microsoft": "^1.0.0",
     "prop-types": "^15.8.1",
+    "quick-lru": "^7.3.0",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",
     "reflect-metadata": "^0.1.13",

packages/apollo-collaboration-server/package.json

@@ -237,6 +237,9 @@ export class ChangesService {
           await this.refSeqChunkModel
             .deleteMany({ $and: [{ status: -1, user: uniqUserId }] })
             .exec()
+          if (error instanceof Error) {
+            throw error
+          }
           throw new UnprocessableEntityException(String(error))
         }
       })

packages/apollo-collaboration-server/src/changes/changes.service.ts

@@ -100,12 +100,26 @@ export class ChecksService {
       if (!c) {
         throw new Error(`Check "${check.name}" not registered`)
       }
-      const result = await c.checkFeature(
-        flatDoc,
-        (start: number, end: number) => {
+      // If the feature is small enough, just fetch its sequence once and
+      // substring that to avoid slow disk reads
+      let featureSequence: string | undefined
+      if (doc.max - doc.min <= 1_000_000) {
+        featureSequence = await this.getSequence({
+          start: doc.min,
+          end: doc.max,
+          featureDoc: doc,
+        })
+      }
+      // eslint-disable-next-line unicorn/consistent-function-scoping
+      const getSequence = (start: number, end: number) => {
+        if (!featureSequence || start < doc.min || end > doc.max) {
           return this.getSequence({ start, end, featureDoc: doc })
-        },
-      )
+        }
+        return Promise.resolve(
+          featureSequence.slice(start - doc.min, end - doc.min),
+        )
+      }
+      const result = await c.checkFeature(flatDoc, getSequence)
       if (result.length > 0) {
         await this.checkResultModel.insertMany(result)
       }

packages/apollo-collaboration-server/src/checks/checks.service.ts

@@ -14,7 +14,7 @@ import {
 } from '@nestjs/common'
 import { ConfigService } from '@nestjs/config'
 import { InjectModel } from '@nestjs/mongoose'
-import { type GenericFilehandle, LocalFile } from 'generic-filehandle'
+import { type GenericFilehandle, LocalFile } from 'generic-filehandle2'
 import { Model } from 'mongoose'
 
 import { CreateFileDto } from './dto/create-file.dto.js'

packages/apollo-collaboration-server/src/files/files.service.ts

@@ -20,7 +20,7 @@ import type {
   FilehandleOptions,
   GenericFilehandle,
   Stats,
-} from 'generic-filehandle'
+} from 'generic-filehandle2'
 
 interface FileUpload {
   originalname: string
@@ -116,20 +116,11 @@ export class LocalFileGzip implements GenericFilehandle {
     this.contents = fhPromise.then((fh) => unzip(fh))
   }
 
-  public async read(
-    buffer: Buffer,
-    offset = 0,
-    length: number,
-    position = 0,
-  ): Promise<{ bytesRead: number; buffer: Buffer }> {
+  public async read(length: number, position = 0): Promise<Buffer> {
+    const buffer = Buffer.alloc(length)
     const unzippedContents = await this.contents
-    const bytesRead = unzippedContents.copy(
-      buffer,
-      offset,
-      position,
-      position + length,
-    )
-    return { bytesRead, buffer }
+    unzippedContents.copy(buffer, 0, position, position + length)
+    return buffer
   }
 
   public async readFile(

packages/apollo-collaboration-server/src/files/filesUtil.ts

@@ -7,6 +7,14 @@ export class GlobalExceptionsFilter extends BaseExceptionFilter {
 
   catch(exception: unknown, host: ArgumentsHost) {
     this.logger.error(exception)
+    if (exception instanceof Error) {
+      if (exception.cause) {
+        this.logger.error(exception.cause)
+      }
+      if (exception.stack) {
+        this.logger.error(exception.stack)
+      }
+    }
     super.catch(exception, host)
   }
 }

packages/apollo-collaboration-server/src/global-exceptions.filter.ts

@@ -11,14 +11,36 @@ import {
 import { BgzipIndexedFasta, IndexedFasta } from '@gmod/indexedfasta'
 import { Injectable, Logger } from '@nestjs/common'
 import { InjectModel } from '@nestjs/mongoose'
-import { RemoteFile } from 'generic-filehandle'
+import { type GenericFilehandle, RemoteFile } from 'generic-filehandle2'
 import { Model } from 'mongoose'
+import QuickLRU from 'quick-lru'
 
 import { AssembliesService } from '../assemblies/assemblies.service.js'
 import { FilesService } from '../files/files.service.js'
 
 import { GetSequenceDto } from './dto/get-sequence.dto.js'
 
+interface AdapterCache {
+  adapter: IndexedFasta | BgzipIndexedFasta
+  fileHandles: GenericFilehandle[]
+}
+
+const adapterLRU = new QuickLRU<string, AdapterCache>({
+  maxSize: 100,
+  maxAge: 24 * 60 * 60 * 1000,
+  onEviction(key, adapter) {
+    const { fileHandles } = adapter
+    for (const fileHandle of fileHandles) {
+      void fileHandle.close()
+    }
+  },
+})
+
+const refSeqDocLRU = new QuickLRU<string, RefSeqDocument>({
+  maxSize: 100,
+  maxAge: 24 * 60 * 60 * 1000,
+})
+
 @Injectable()
 export class SequenceService {
   constructor(
@@ -35,9 +57,15 @@ export class SequenceService {
   private readonly logger = new Logger(SequenceService.name)
 
   async getSequence({ end, refSeq: refSeqId, start }: GetSequenceDto) {
-    const refSeq = await this.refSeqModel.findById(refSeqId)
+    let refSeq: RefSeqDocument | null | undefined = refSeqDocLRU.get(
+      String(refSeqId),
+    )
     if (!refSeq) {
-      throw new Error(`RefSeq "${refSeqId}" not found`)
+      refSeq = await this.refSeqModel.findById(refSeqId)
+      if (!refSeq) {
+        throw new Error(`RefSeq "${refSeqId}" not found`)
+      }
+      refSeqDocLRU.set(String(refSeqId), refSeq)
     }
 
     const { assembly, chunkSize, name } = refSeq
@@ -47,17 +75,34 @@ export class SequenceService {
 
     if (assemblyDoc.externalLocation) {
       const { fa, fai, gzi } = assemblyDoc.externalLocation
+      const adapterCacheEntry = adapterLRU.get(fa)
+      let sequenceAdapter = adapterCacheEntry?.adapter
 
-      const sequenceAdapter = gzi
-        ? new BgzipIndexedFasta({
-            fasta: new RemoteFile(fa, { fetch }),
-            fai: new RemoteFile(fai, { fetch }),
-            gzi: new RemoteFile(gzi, { fetch }),
+      if (!sequenceAdapter) {
+        const fastaHandle = new RemoteFile(fa, { fetch })
+        const faiHandle = new RemoteFile(fai, { fetch })
+        if (gzi) {
+          const gziHandle = new RemoteFile(gzi, { fetch })
+          sequenceAdapter = new BgzipIndexedFasta({
+            fasta: fastaHandle,
+            fai: faiHandle,
+            gzi: gziHandle,
           })
-        : new IndexedFasta({
-            fasta: new RemoteFile(fa, { fetch }),
-            fai: new RemoteFile(fai, { fetch }),
+          adapterLRU.set(fa, {
+            adapter: sequenceAdapter,
+            fileHandles: [fastaHandle, faiHandle, gziHandle],
           })
+        } else {
+          sequenceAdapter = new IndexedFasta({
+            fasta: fastaHandle,
+            fai: faiHandle,
+          })
+          adapterLRU.set(fa, {
+            adapter: sequenceAdapter,
+            fileHandles: [fastaHandle, faiHandle],
+          })
+        }
+      }
       const sequence = await sequenceAdapter.getSequence(name, start, end)
       if (sequence === undefined) {
         throw new Error('Sequence not found')
@@ -67,29 +112,37 @@ export class SequenceService {
 
     if (assemblyDoc.fileIds?.fai) {
       const { fa: faId, fai: faiId, gzi: gziId } = assemblyDoc.fileIds
-      const faDoc = await this.fileModel.findById(faId)
-      if (!faDoc) {
-        throw new Error(`No checksum for file document ${faId}`)
-      }
+      const adapterCacheEntry = adapterLRU.get(String(faId))
+      let sequenceAdapter = adapterCacheEntry?.adapter
+      if (!sequenceAdapter) {
+        const faDoc = await this.fileModel.findById(faId)
+        if (!faDoc) {
+          throw new Error(`No checksum for file document ${faId}`)
+        }
 
-      const faiDoc = await this.fileModel.findById(faiId)
-      if (!faiDoc) {
-        throw new Error(`File document not found for ${faiId}`)
-      }
+        const faiDoc = await this.fileModel.findById(faiId)
+        if (!faiDoc) {
+          throw new Error(`File document not found for ${faiId}`)
+        }
 
-      const gziDoc = await this.fileModel.findById(gziId)
-      if (!gziDoc) {
-        throw new Error(`File document not found for ${gziId}`)
-      }
+        const gziDoc = await this.fileModel.findById(gziId)
+        if (!gziDoc) {
+          throw new Error(`File document not found for ${gziId}`)
+        }
 
-      const fasta = this.filesService.getFileHandle(faDoc)
-      const fai = this.filesService.getFileHandle(faiDoc)
-      const gzi = gziId ? this.filesService.getFileHandle(gziDoc) : undefined
-      const sequenceAdapter = gziId
-        ? new BgzipIndexedFasta({ fasta, fai, gzi })
-        : new IndexedFasta({ fasta, fai })
+        const fasta = this.filesService.getFileHandle(faDoc)
+        const fai = this.filesService.getFileHandle(faiDoc)
+        const gzi = gziId ? this.filesService.getFileHandle(gziDoc) : undefined
+        sequenceAdapter = gziId
+          ? new BgzipIndexedFasta({ fasta, fai, gzi })
+          : new IndexedFasta({ fasta, fai })
+        const fileHandles = [fasta, fai]
+        if (gzi) {
+          fileHandles.push(gzi)
+        }
+        adapterLRU.set(String(faId), { adapter: sequenceAdapter, fileHandles })
+      }
       const sequence = await sequenceAdapter.getSequence(name, start, end)
-      await Promise.all([fasta.close(), fai.close(), gzi?.close()])
       if (sequence === undefined) {
         throw new Error('Sequence not found')
       }

packages/apollo-collaboration-server/src/sequence/sequence.service.ts

@@ -24,7 +24,7 @@
     "@nestjs/common": "^10.1.0",
     "@nestjs/core": "^10.1.0",
     "@types/node": "^24.10.9",
-    "generic-filehandle": "^3.0.0",
+    "generic-filehandle2": "^2.0.18",
     "mongoose": "^6.12.0",
     "typescript": "^5.5.3"
   },

packages/apollo-common/package.json

@@ -15,7 +15,7 @@ import type {
 } from '@apollo-annotation/schemas'
 import type { GFF3Feature } from '@gmod/gff'
 import type { LoggerService } from '@nestjs/common'
-import type { GenericFilehandle } from 'generic-filehandle'
+import type { GenericFilehandle } from 'generic-filehandle2'
 import type { ClientSession, Model } from 'mongoose'
 
 export interface LocalGFF3DataStore {