qwc-open/asiakirjat

Watch

Fork 0

Three silent failures: an unreported mkdir, a capped delete, an unbounded batch #169

Open

qwc wants to merge 4 commits from fix/startup-and-index-robustness into main

qwc commented

2026-10-02 11:21:47 +02:00

Owner

Three independent findings from debugging a Kubernetes startup failure. Each is a commit of its own and they can be reviewed separately.

1. A directory we cannot create says so (`e3a9bb8`)

Both startup MkdirAll calls discarded their error. The database one let the failure fall through to the SQLite driver, which reports SQLITE_CANTOPEN as:

pinging database: unable to open database file: out of memory (14)

The out of memory is the driver's fallback errmsg string, not a diagnosis (vendor/modernc.org/sqlite/conn.go:626-639 formats errors as errstr: errmsg (rc)). It sends the reader after RAM and PVC sizes. The actual cause was a volume mounted over /app/data arriving root-owned while the container runs as uid 1000. The storage directory had the same hole and failed even later — on first upload.

Deriving the directory also needed a driver check: filepath.Dir on a PostgreSQL or MySQL DSN treats a connection string as a path, so the old code would create a directory named after a fragment of the DSN, password included. sqliteDataDir returns a directory only for SQLite, and skips in-memory databases and file: URIs with query parameters.

Both resolved absolute paths are now logged at startup, so a DSN relative to an unexpected working directory is visible without a shell in the container.

2. Deleting from the search index deletes all of it (`b500468`)

Both deletion paths found their documents with a single search request of Size = 10000 and ignored everything past that page. On an index holding more than 10000 documents:

DeleteVersion deleted whichever of a version's pages happened to land in the first page of an unordered match-all result and left the rest searchable, so pages of a deleted version kept appearing in search results, linking to documentation that is gone. Especially visible now that #164 removes storage on project delete.
ReindexAllWithProgress could not clear the documents a reindex exists to clear, so "rebuild from scratch" was not one. It also discarded the search error and ignored the batch error, so it could report success having done nothing.

A search request always has a size, so it is the wrong tool. The fix walks the index reader (DocIDReaderAll) instead: no cap, no scoring, each document visited once. project_id/version_id are mapped with Index = false, so they cannot be queried — hence the id-prefix match stays, now over a complete enumeration.

The reindex clear is now a hard failure, since the state it leaves behind on partial success is exactly the one a reindex is meant to repair.

3. Indexing a version no longer holds all of its text in memory (`316ef41`)

IndexVersion accumulated one bleve batch across an entire version and flushed it once at the end. A batch holds the full extracted text of every document in it, so peak memory was a function of how much documentation somebody uploaded — a large generated tree, or a PDF contributing one document per page, grew it without bound. That peak lands immediately after an upload, on an instance sized for steady state. Now flushed every 500 documents, inside the PDF page loop as well as per file.

Verification

The deletion tests seed 10001 documents, one past the old page, and fail against the previous implementation with exactly one document surviving. Confirmed before writing the fix.
The batch tests index indexBatchSize-1, indexBatchSize and indexBatchSize+1 files, so a document stranded in an unflushed batch at a boundary fails them, plus the empty-directory case where there is no final batch.
sqliteDataDir is tested across relative, absolute, bare-filename, file: URI, :memory:, and PostgreSQL/MySQL DSN forms; ensureDir for nested creation, idempotence, and that the error names both the purpose and the path.
Full suite green: go test ./... (12 packages) and npm test (19 tests).
search-indexing.md described the deletion step as a query, which is what it used to be and the reason it was incomplete; updated to describe the walk and the batching.

🤖 Generated with Claude Code

Three independent findings from debugging a Kubernetes startup failure. Each is a commit of its own and they can be reviewed separately. ## 1. A directory we cannot create says so (`e3a9bb8`) Both startup `MkdirAll` calls discarded their error. The database one let the failure fall through to the SQLite driver, which reports `SQLITE_CANTOPEN` as: ``` pinging database: unable to open database file: out of memory (14) ``` The `out of memory` is the driver's fallback `errmsg` string, not a diagnosis (`vendor/modernc.org/sqlite/conn.go:626-639` formats errors as `errstr: errmsg (rc)`). It sends the reader after RAM and PVC sizes. The actual cause was a volume mounted over `/app/data` arriving root-owned while the container runs as uid 1000. The storage directory had the same hole and failed even later — on first upload. Deriving the directory also needed a driver check: `filepath.Dir` on a PostgreSQL or MySQL DSN treats a connection string as a path, so the old code would create a directory named after a fragment of the DSN, **password included**. `sqliteDataDir` returns a directory only for SQLite, and skips in-memory databases and `file:` URIs with query parameters. Both resolved absolute paths are now logged at startup, so a DSN relative to an unexpected working directory is visible without a shell in the container. ## 2. Deleting from the search index deletes all of it (`b500468`) Both deletion paths found their documents with a single search request of `Size = 10000` and ignored everything past that page. On an index holding more than 10000 documents: - `DeleteVersion` deleted whichever of a version's pages happened to land in the first page of an unordered match-all result and left the rest searchable, so **pages of a deleted version kept appearing in search results**, linking to documentation that is gone. Especially visible now that #164 removes storage on project delete. - `ReindexAllWithProgress` could not clear the documents a reindex exists to clear, so "rebuild from scratch" was not one. It also discarded the search error and ignored the batch error, so it could report success having done nothing. A search request always has a size, so it is the wrong tool. The fix walks the index reader (`DocIDReaderAll`) instead: no cap, no scoring, each document visited once. `project_id`/`version_id` are mapped with `Index = false`, so they cannot be queried — hence the id-prefix match stays, now over a complete enumeration. The reindex clear is now a hard failure, since the state it leaves behind on partial success is exactly the one a reindex is meant to repair. ## 3. Indexing a version no longer holds all of its text in memory (`316ef41`) `IndexVersion` accumulated one bleve batch across an entire version and flushed it once at the end. A batch holds the full extracted text of every document in it, so peak memory was a function of how much documentation somebody uploaded — a large generated tree, or a PDF contributing one document per page, grew it without bound. That peak lands immediately after an upload, on an instance sized for steady state. Now flushed every 500 documents, inside the PDF page loop as well as per file. ## Verification - The deletion tests seed 10001 documents, one past the old page, and **fail against the previous implementation** with exactly one document surviving. Confirmed before writing the fix. - The batch tests index `indexBatchSize-1`, `indexBatchSize` and `indexBatchSize+1` files, so a document stranded in an unflushed batch at a boundary fails them, plus the empty-directory case where there is no final batch. - `sqliteDataDir` is tested across relative, absolute, bare-filename, `file:` URI, `:memory:`, and PostgreSQL/MySQL DSN forms; `ensureDir` for nested creation, idempotence, and that the error names both the purpose and the path. - Full suite green: `go test ./...` (12 packages) and `npm test` (19 tests). - `search-indexing.md` described the deletion step as a query, which is what it used to be and the reason it was incomplete; updated to describe the walk and the batching. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 4 commits

2026-10-02 11:21:47 +02:00

A directory we cannot create says so e3a9bb8fd4

Both startup MkdirAll calls discarded their error. When the directory could not
be created, the database one let the failure fall through to the SQLite driver,
which reports SQLITE_CANTOPEN as

    pinging database: unable to open database file: out of memory (14)

The "out of memory" is the driver's fallback errmsg string, not a diagnosis, and
it sends the reader after RAM and PVC sizes instead of at the mount. The real
cause in the case that prompted this was a Kubernetes volume mounted over
/app/data arriving owned by root while the container runs as uid 1000. The
storage directory had the same hole, and failed even later — on first upload.

Report both, naming the purpose and the path.

Deriving the directory also needed a driver check. filepath.Dir on a PostgreSQL
or MySQL DSN treats a connection string as a path, so the old code would create
a directory named after a fragment of the DSN, password included. sqliteDataDir
returns a directory only for SQLite, and skips in-memory databases and file:
URIs with query parameters.

The resolved absolute path of both directories is now logged at startup, so a
DSN that is relative to a working directory you did not expect is visible
without a shell in the container.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

Deleting from the search index deletes all of it b5004681b4

Both deletion paths found the documents to remove with a single search request
of size 10000 and ignored everything past that page. On an index holding more
than 10000 documents:

- DeleteVersion deleted whichever of the version's pages happened to fall in
  the first page of an unordered match-all result, and left the rest
  searchable. Pages of a deleted version therefore kept appearing in search
  results, now linking to documentation that is gone.
- ReindexAllWithProgress could not clear the documents a reindex exists to
  clear, so "rebuild from scratch" was not one.

A search request always has a size, so it is the wrong tool. Walk the index
reader instead: no cap, no scoring, each document visited once. Deletions flush
in batches of 500 rather than accumulating one batch over the whole index.

The reindex clear is now a hard failure. It used to discard the search error
entirely and then ignore the batch error, so a reindex could report success
having done nothing at all — and the state it leaves behind is exactly the one
a reindex is meant to repair.

The tests seed 10001 documents, one past the old page, and fail against the
previous implementation with a single document surviving.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

Indexing a version no longer holds all of its text in memory 316ef41bf6

IndexVersion accumulated one bleve batch across an entire version and flushed
it once at the end. A batch holds the full extracted text of every document in
it, so peak memory was a function of how much documentation somebody uploaded,
not of anything we control: a large generated tree, or a PDF contributing one
document per page, grew it without bound. That peak lands right after an
upload, on an instance whose memory limit was sized for steady state.

Flush every 500 documents instead, inside the PDF page loop as well as per
file, since one PDF can contribute more documents than a whole directory of
HTML.

The tests index indexBatchSize-1, indexBatchSize and indexBatchSize+1 files, so
a document stranded in an unflushed batch at a boundary fails them, and check
the empty-directory case where there is no final batch to flush.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

Document how the index batches and clears

CI / test (pull_request) Successful in 1m20s

Details

CI / build (pull_request) Successful in 45s

Details

CI / docker (pull_request) Has been skipped

Details

cc38eb4ac4

The search internals page described the deletion step as a query, which is what
it used to be and the reason it was incomplete. Describe the walk, say why a
search cannot do the job, and note the batch flushing and the reindex clear
being a hard failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

CI / test (pull_request) Successful in 1m20s

Details

CI / build (pull_request) Successful in 45s

Details

CI / docker (pull_request) Has been skipped

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'fix/startup-and-index-robustness:fix/startup-and-index-robustness'

git switch 'fix/startup-and-index-robustness'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'main'

git merge --no-ff 'fix/startup-and-index-robustness'

git switch 'fix/startup-and-index-robustness'

git rebase 'main'

git switch 'main'

git merge --ff-only 'fix/startup-and-index-robustness'

git switch 'fix/startup-and-index-robustness'

git rebase 'main'

git switch 'main'

git merge --no-ff 'fix/startup-and-index-robustness'

git switch 'main'

git merge --squash 'fix/startup-and-index-robustness'