Repository navigation
Conversation
…N-001)
GOOS=windows failed on three files using Unix-only syscalls: syscall.Getrusage
for the CPU sampler, and syscall.Statfs for disk space in two separate places.
The inventory was smaller than the ticket assumed. Running go vet rather than
go build — vet compiles test files too — showed clients/go/mddb, mddb-cli, test
and tools/bench already passing for windows/amd64. Only mddbd failed.
Each syscall is now one signature with two implementations behind a build tag:
diskSpace(path) (avail, total uint64, err error) statfs / GetDiskFreeSpaceEx
processCPUTime() (userNs, systemNs int64, err error) getrusage / GetProcessTimes
No call site changed platform behaviour. diskUsage and freeBytes had been
running near-identical block arithmetic in two files, each with its own #nosec
annotation; they now derive from one primitive, which is why the nosec count
drops from 203 to 199.
One thing in the Windows implementation looks wrong and is not.
windows.Filetime has a Nanoseconds method, and calling it here would be a bug:
it subtracts the 1601-to-1970 epoch offset because it converts timestamps,
while GetProcessTimes returns durations. A second of CPU would read as roughly
minus 369 years — a wrong number, not a crash, in a metric nobody double-checks.
A test asserts that method is wrong for this input, so an edit reaching for the
obvious call gets stopped by a failure rather than shipping a plausible graph.
The failure branches are unreachable on a working machine — getrusage does not
fail for RUSAGE_SELF — so the syscalls are injected through a variable, the same
pattern TEST-004 used for production timings. That takes the platform layer to
100% statement coverage, and all four files are now tracked coverage areas; the
Windows pair reads as "not in profile" on Linux and will be checked by the
windows-latest job from WIN-004.
Fixed while here:
* make build was documented in the README and did not exist. Added, next to
the new build-windows.
* make version printed a hardcoded 2.11.4 — through two releases. It is a
fourteenth version source that scripts/check-version.sh does not know about,
while the thirteen it does guard stayed in step. It now reads the version
from services/mddbd/main.go rather than keeping a copy.
This is not Windows support, and the docs now say so. The binaries build and
link; no CI job has run a test on Windows and no release ships a Windows
artifact. Replacing a file another process holds open behaves differently there
(WIN-002) and Unix domain sockets compile without being exercised. WSL2 remains
the supported route.
go vet ./... incl. tests, all six release targets windows/amd64 linux/amd64
linux/arm64 freebsd/amd64
darwin/amd64 darwin/arm64
go test ./... ok, 32 packages
gofmt -s / golangci-lint clean, 0 issues
gosec 7, all in generated proto
make build-windows mddbd.exe 29.5 MB,
mddb-cli.exe 7.8 MB, PE32+
platform layer coverage 100%
… on Windows (WIN-002, SEC-017, SEC-018)
WIN-002 planned a platform-split replaceFile helper, because os.Rename is
widely documented as not overwriting on Windows. It does. Go 1.27:
src/os/file_windows.go rename() -> windows.Rename()
internal/syscall/windows/… Rename() -> MoveFileEx(from, to,
MOVEFILE_REPLACE_EXISTING)
The helper would have wrapped MoveFileEx(REPLACE_EXISTING) in a call to
MoveFileEx(REPLACE_EXISTING). It was not written. A split that splits nothing
is maintenance cost with no benefit.
The other half of the premise is real: syscall.Open on Windows uses
FILE_SHARE_READ|FILE_SHARE_WRITE and no FILE_SHARE_DELETE, so an open file can
be neither renamed over nor deleted. Auditing that found four os.Rename sites
and nine os.Remove sites, every one of which already closes first. Nothing was
broken there — but three things behind it were.
SEC-017: restore via the MCP tool destroyed the database with no way back.
restore_backup.go calls itself "the single implementation behind POST
/v1/restore and the gRPC Restore RPC" and was, for those two. The MCP tool was
a third entry point with its own close-copy-reopen: no check that the file was
a database, no restore lock, no rollback, no cache rebuild, no binlog reset.
Any readable file inside the backup directory overwrote the live database; it
did not have to be a database, because the copy succeeds on bytes and only the
reopen fails, by which point the original is gone. Measured before the fix:
--- FAIL: TestDirectClientRestore_FailureLeavesTheDatabaseServing
write after a failed MCP restore failed: database not open
Note how it presented: the read after the failure still succeeded from cache.
The dead database looked healthy until the first write.
SEC-018: the replication snapshot install was a fourth hand-rolled swap. It
renamed the received file over the live database without checking it was one,
so a truncated stream was indistinguishable from a good one until the reopen —
after the follower's data was gone. Every failure left a closed handle until
process restart. It also reopened with hand-written bolt options rather than
getOptimizedBoltOptions(), so a follower that had ever taken a snapshot ran
with a different mmap and lock configuration than the rest of the process.
All three now share swapDatabase(source, install): validate, close, move the
live file aside, install, reopen, roll back on any failure. The callers differ
only in install — restore copies, because a backup must survive being restored
from; replication renames, because the snapshot is a temp file the follower
owns and copying a multi-gigabyte file in order to delete it is waste. There is
a test for that distinction, because "copies" and "moves" are indistinguishable
from outside until a disk fills.
SEC-018, found while testing the above: the rollback had never run. Coverage
put the entire rollback closure and both its call sites at zero executions.
Every existing failure test supplied a source that validation rejected, so the
swap returned before touching the live file — the code whose only job is to
save the database when a restore fails had not executed since it was written
for SEC-015. Reaching it needs a source that validates and an install that then
fails, which is what making install a parameter allows. Two tests now do:
64.5% -> 83.9%. Both mutation-checked; removing the reopen after rollback fails
them with "database not writable: database not open".
Also dropped the os.Remove(s.Path) that ran immediately before the restoring
rename. Redundant, since rename replaces on every platform, and it opened a
window in which no database was at s.Path at all.
WIN-002's one real platform change: unix domain sockets are refused on Windows.
This is not a build problem — Windows 10 supports AF_UNIX and net.Listen
succeeds. os.Chmod there toggles the read-only attribute and cannot express
"owner only", so the socket would bind and serve with the access restriction
the listener documents silently absent. It says so and points at
tcp://127.0.0.1 instead. An unprotected IPC channel that does not admit it is
worse than one that will not start.
go vet ./... incl. tests, all six release targets all pass
go test ./... ok, 32 packages
go test -race -count=2 (restore, replication) ok, 3.3s
gofmt -s / golangci-lint clean, 0 issues
gosec 7, all in generated proto
swapDatabase coverage 64.5% -> 83.9%
SonarCloud was the only red check of thirty on the release PR: 6.2% duplication
on new code against a 3% gate. It was right. The point of SEC-017/SEC-018 is
that all three entry points share one contract, and I had written that as three
near-identical tests, which says the opposite.
They are now one table over the three callers: restore, the MCP tool, the
replication snapshot install. 349 lines to 241, nine subtests, and the
duplicate assertions gone.
Three things came out of doing it.
A test that could not distinguish validation from rollback. Removing the source
validation entirely left the suite green, because the rollback still saves the
database, so "an error, and the database still serves" holds either way. The
difference is that with validation the live handle is never touched at all,
and without it the rollback installs a replacement. The table now asserts s.DB
is the same object it was before the call — the actual contract, "rejected
before the live file was touched", rather than the outcome. Removing the
validation now fails it.
copyFile left an orphan on every failure path. It writes to dst.tmp and renames
into place, and removed the temp file on none of the three ways out. After a
failed restore that orphan is a partial copy of the database, on the filesystem
whose running out of room is the most likely reason the copy failed. Removed in
a defer; both calls are no-ops after success.
Nothing verified that a restore resets the binlog. The branch had never
executed, because every server in the restore suite runs with s.Binlog nil.
Without the reset a follower applies its old LSN stream onto a database that no
longer holds the rows those entries assume. Covered now, and mutation-checked:
removing the reset fails with "oldest LSN = 1 after restore, want 0". That took
restoreFromBackup from 75% to 87.5%.
restore_backup.go is now a tracked coverage area at a floor of 80 (currently
81.2%). It earned that by holding a rollback with zero executions from the day
it was written until SEC-018 measured it.
Also fixed a deadlock I wrote into the refactor: backupNow reads through
DBView, and calling it inside withRestoreLock — which exists to drain and block
exactly those — hangs the test against itself. It runs before the lock now,
with a note saying why. The 180-second timeout is what caught it.
go test ./... ok, 32 packages, 0 FAIL
go test -race -count=2 (swap, restore, copy) ok, 3.3s
go vet ./... incl. tests, 7 targets all pass
gofmt -s / golangci-lint clean, 0 issues
gosec 7, all in generated proto
restore_backup.go coverage 79.7% -> 81.2%
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ring a temp name
CI failed on a test I converted in TEST-004:
embedding_worker_test.go:250: Vector[0] = 0.100000, want 0.4
It waited for the embedding provider's call count to reach 1 and then read the
vector store. Those are two events — the provider returning a vector, and the
worker persisting it — and waiting on the first races the second. It read the
pre-stored 0.1. This is the exact failure mode TEST-004's own notes warn about.
Auditing all twenty-five testsync call sites for the same shape found two more:
* a webhook test incremented its received-counter before unmarshalling the
payload it then asserted on. Two other tests in the same file already had
this the right way round, with a comment saying why; this one was missed.
* internal/audit/exporter.go incremented Failed before setting LastError.
That one is production code: anything polling Stats() — a monitor as much
as a test — could observe a failure recorded with no error attached to it.
The counter is now the last thing to become visible in all three. The remaining
twenty-two are sound: indexqueue increments processed after processJob returns,
and the rest wait on the end state itself.
CodeQL flagged go/path-injection on the os.Remove added in the previous commit.
Suppressing the rule was not an option — it is what caught the missing
permission check on /v1/geo-reindex in 2.12 — so copyFile now takes its temp
file from os.CreateTemp in the destination's directory rather than assembling
dst+".tmp".
That turned out to fix a real bug rather than only satisfy a scanner. A fixed
temp name means two backups requested with the same destination write through
one another's temp file. They still race on the final rename — one wins, and
that is the caller's business — but the winner could publish a blend of both
sources. Mutation-checked: restoring the shared name fails the new concurrency
test four times in five with "destination mixes bytes from more than one
source".
It also caught my own two copyFile tests passing vacuously: they stat'd
dst+".tmp", a name os.CreateTemp no longer produces. They glob dst+".tmp-*"
now, through a helper that says why.
go test ./... ok, 32 packages
go test -race -count=5 (embedding, webhooks, audit, ok
copy, swap, restore)
go vet ./... incl. tests, 7 targets all pass
gofmt -s / golangci-lint clean, 0 issues
gosec 7, all in generated proto
…, again CodeQL raised go/path-injection on copyFile for the fifth time — alerts #19, #20, #56, #57 and now #60 — each at a different line, because each edit to the function moves it. All five are the same false positive and all five are dismissed with the same reasoning, which until now existed only in a comment field in the GitHub UI. The SARIF flow shows why it keeps coming back: CodeQL traces dst from the backup handler's query parameter through safeBackupPath and does not model that function as a barrier. It sees the filepath.Clean calls — which correctly are not barriers, since Clean does not stop traversal — and not the filepath.Rel check against ".." that follows them on symlink-resolved paths. Excluding the rule is not an option: it is what found the missing permission check on /v1/geo-reindex in 2.12, which was a real bug. query-filters can exclude a rule but not a rule within a file, so a dismissal is the only mechanism available. The reasoning now lives in two places that survive review: a comment at the call site, where whoever next moves the line will read it, and a note in the CodeQL config next to the two rules that ARE filtered there, saying why this one cannot be.
Until now everything known about MDDB on Windows came from GOOS=windows go vet
run on Linux: that the code compiles and type-checks, and nothing about whether
GetDiskFreeSpaceEx returns what diskSpace assumes or whether a rename lands
where the restore contract expects. This job is what can answer that.
Deliberately narrower than the Linux job — no lint, gosec, coverage upload or
fuzzing. Those are platform-independent and running them twice buys minutes,
not information. No -race either: the detector finds data races, which are a
property of the code rather than the operating system, and the Linux job
already runs the whole suite under it.
Four things in the suite had to change first, because they would have failed
for reasons that are not about MDDB:
* Four auth test files opened their bolt database at a fixed
/tmp/test_auth_<name>.db. That path does not exist on Windows, and even on
Unix it outlived the run — a second run inherited the first one's database.
They use t.TempDir now, with the Close registered as a t.Cleanup as well as
in the returned cleanup: Windows refuses to remove a directory holding an
open file, so a test that forgot `defer cleanup()` would fail the temp
directory removal rather than merely leak. 132 call sites keep their
signatures.
* The traversal tests passed "/etc/passwd" as the absolute-path case.
filepath.IsAbs says false for that on Windows, where an absolute path needs
a volume — the jail would have treated it as a relative name, joined it
inside itself and correctly accepted it. The test would have stopped
covering the absolute case without ever going red. They now build a path
outside the jail for whatever platform is running.
* The three UDS tests that actually create a socket are skipped on Windows,
where MDDB refuses that listener by design (WIN-002). Not a blanket skip:
listen_addr_windows_test.go asserts the refusal from the other side, and
that the message names tcp://127.0.0.1 rather than saying "unsupported".
* A guard for source encoding. PowerShell's `>` and Out-File default to
UTF-16 LE with a BOM, and Go answers a file like that with "illegal UTF-8
encoding" — an error that names no cause. .gitattributes normalises line
endings and says nothing about encoding. scripts/check-source-encoding.sh
checks the leading bytes of all 1165 tracked text files; its own suite
covers nine cases, including that a UTF-8 BOM is rejected in Go source
(where it silently breaks a leading //go:build) and tolerated in Markdown.
Wired into the Makefile as check-source-encoding / test-source-encoding, and
into the `ci` target alongside the other guards.
GOOS=windows GOARCH=amd64 go vet ./... incl. tests pass
go vet ./... (linux) pass
go test (auth, traversal, listener, backup, restore) ok
scripts/tests/test-source-encoding.sh 9/9
scripts/check-action-pins.sh 138 references pinned
…ase got wrong
31 of 32 packages passed on windows-latest at the first attempt. The ninth
failure in the remaining one repaid the job on its own.
A correction first. WIN-001 said calling windows.Filetime.Nanoseconds on a
GetProcessTimes duration would read as "roughly minus 369 years". It does not.
The epoch subtraction reaches about -1.16e19, an int64 stops at -9.22e18, so
the multiply by 100 wraps and the method returns 6802270474709551616. CI
reported that number and the arithmetic confirms it exactly. The conclusion is
unchanged but understated: a negative CPU time is obviously broken, while a
large positive one looks like a reading. My test asserted "goes negative" and
failed, which is the best outcome this job could have had. Corrected in the
code comment, the test, the CHANGELOG and the ticket.
Two more instances of waiting on a signal that arrives before the asserted
state, making five this release. The embedding worker writes the vector store
and then the in-memory index; a test waited on the store and asserted the
index. A bulk job's status is set before its terminal event reaches SSE
subscribers; a test waited on the status and asserted the stream ended on the
event. Both windows exist on Linux too — Windows only widened them.
Two test helpers closed the handle captured at setup, and a restore replaces
it. On Unix the file went away regardless; on Windows the temp directory could
not be removed around the still-open live database. newTestServer was worse: it
used os.CreateTemp in the system temp directory and ignored its own os.Remove
error, leaking a database per test in silence. It uses t.TempDir now, which
fails loudly — which is how the wrong-handle bug surfaced.
Three tests assert POSIX semantics — /proc/self/mountinfo parsing, and a
directory made unwritable through mode bits that do not restrict the owner on
Windows. They skip there, each with its reason, per WIN-004's requirement that
no test is disabled blind.
And one finding that contradicts this release. WIN-002 concluded os.Rename
replaces an existing file on every platform Go supports. True, but not under
concurrency: Windows must delete the target first, and deletion there is not
immediate — a file with an open handle is marked for deletion and persists
until the last one closes. A rename landing in that window fails with "Access
is denied". Concurrent copies to one destination are a caller error Windows
reports and Unix quietly resolves. The invariant that holds on both, and what
the test now asserts, is that the destination is never a blend of two sources.
go test ./... ok, 32 packages, 0 FAIL
go test -race -count=3 (the tests changed here) ok
go vet ./... incl. tests, 7 targets all pass
gofmt -s / golangci-lint clean, 0 issues
gosec 7, all in generated proto
The second Windows run moved the failures out of mddbd — all 32 of its packages
passed, along with the shared Go client — and into mddb-cli's self-update tests.
Both were assertions about POSIX rather than about MDDB:
* an execute bit. Windows has none; what makes a file runnable there is its
extension, and Go synthesises Mode().Perm() from the read-only attribute
alone. That the mode is carried over from the binary being replaced is
asserted by TestReplaceBinaryPreservesTheExistingMode, which does run there.
* a directory made unwritable with os.Chmod, which on Windows toggles the
read-only attribute and does not take write access from the owner. The
update rightly succeeded. Denying it needs an ACL, which is a different
subject from the one that test is about.
The interesting part is what did not fail. ReplaceBinary works on Windows,
which was not obvious: you cannot overwrite or delete a running executable
there. It renames the running binary aside and then moves the new one into
place, and renaming a running executable IS permitted — so the sequence written
for atomicity on Unix turns out to be the only sequence that works on Windows
either. Nobody designed that; the test found it.
go test ./... (mddb-cli, linux) ok
GOOS=windows GOARCH=amd64 go vet ./... pass
The README listed installation commands and left the reader to infer coverage.
The table now separates two things that are easy to conflate:
* macOS and FreeBSD ship release binaries that no CI job ever executes. They
are built and vetted on every push, test files included, which catches a
platform split that fails to compile — the FreeBSD build broke once on a
Statfs_t field width, and that is what this catches. It does not catch a
syscall that compiles and returns the wrong thing.
* Windows is the opposite: the full suite runs there on every push as of 2.13,
and no release ships a Windows binary. Build from source.
Also states that unix domain sockets are refused on Windows, since that is a
capability difference a reader planning a deployment needs before they plan it,
not after.
Green on three consecutive runs. The job is what turned "compiles for Windows" into "passes its tests on Windows", and it earned itself on the first run by finding nine failures — none of them bugs in MDDB's production code, seven of them real bugs in tests and helpers, and six of those present on Linux too.
…ith numbers
A downstream fork's notes report RSS going from 42 MB to 153 MB under load and
file it as a memory concern. That number cannot decide anything on its own.
RSS includes bbolt's memory map of the database file: those pages grow with the
data, are file-backed, and the kernel reclaims them under pressure. A process
holding a 160 MB database shows a couple of hundred MB of RSS while using
almost none of it as heap.
tools/bench/soak sustains a mixed workload and samples RSS beside HeapInuse,
because separating those two is the entire question. It takes heap profiles at
the start, the middle and the end — three rather than two, because a
start-to-end diff cannot distinguish "grew early then settled" from "grew
throughout", and those have opposite verdicts.
Measured: 45 minutes, 1,261,570 operations across adds, updates, reads, keyword
and hybrid search, zero errors, against a database that grew to 160 MB.
RSS 190 MB -> 247 MB (+57 MB, flat over the last 10 minutes)
HeapInuse 43 MB -> 47 MB (median of first and last quarter)
Sys 80 MB -> 89 MB (flat from minute 10)
goroutines 38 -> 42 (no trend)
The profiles settle it. The largest allocator, embedding.float64sToFloat32s,
gained 6.7 MB across the first half and lost 2.6 MB across the second; FTS
search allocation ended below where it started. Total heap delta was 16.5 MB
spread over 75 nodes with no node above 4 MB. A leak does not shrink.
Verdict: no leak. The fork's observation was probably accurate and its
conclusion wrong — their measurement did not separate heap from RSS, so it
could not have distinguished them, and neither could we by looking at RSS.
The existing "High memory usage" entry in docs/DEPLOYMENT.md is replaced rather
than duplicated: it now explains the distinction, gives the command that reads
memoryHeap, carries these numbers, and says to watch the heap and the goroutine
count rather than RSS.
Found on the way and filed as DOC-018, not fixed here: /v1/get and /v1/delete
answer a missing document with 400 rather than 404. The first soak run counted
40% "errors" for that reason alone. Changing it breaks any client keying on 400,
so it needs sign-off rather than a quiet fix.
…ilt from the workspace
|
❌ The last analysis has failed. |
…CH-011)
The build tags split vector maths two ways — NEON through cgo on arm64, pure Go
everywhere else — and there was no third branch. On amd64, which is most of the
servers this runs on, every vector comparison went through a scalar loop. The
file is called vector_math_scalar.go, which reads like a fallback and was the
production path for the dominant architecture.
Measured with the benchmark the original observation used, dims=768, median of
five runs of three seconds:
candidates scalar AVX2
1,000 6.8 GB/s 43.0 GB/s 6.3x
10,000 6.7 GB/s 36.0 GB/s 5.4x
100,000 6.8 GB/s 24.8 GB/s 3.7x
The shape matters as much as the multiplier. The scalar path was flat: the same
6.8 GB/s whether the working set fit in cache or was fifteen times larger.
Memory-bound code does not behave that way. It was bound by instruction
throughput — which explains both earlier attempts to speed this up, a batch
kernel that measured within noise of the loop and a hoisted query norm that
predicted 33% and delivered 1.5%. Both cut arithmetic over the same bytes. AVX2
cuts instructions per byte, and the result now degrades with size the way
memory-bound code should.
Go assembly rather than cgo, decided by one line in release.yml: releases build
with CGO_ENABLED=0, so a cgo kernel would compile locally and never ship. The
search path also compares one vector at a time — under a microsecond for 768
dimensions — so a cgo boundary crossing would be a visible fraction of the work.
Selection is a runtime CPU check rather than a build tag, because there is one
amd64 build of MDDB and it has to start on a machine without AVX2. That fallback
is tested by taking it deliberately, which is the only way to exercise it
without different hardware.
On correctness, the ticket asked for agreement within 1e-5 and I wrote that
test; AVX2 failed two cases. Before loosening anything I ran the same test
against the scalar build, which failed four. The kernel is more accurate than
the loop it replaces — eight accumulators each carry an eighth of the rounding
error — so the test was wrong, not the code. The failing cases were near-total
cancellation: 384 products of magnitude 100 summing to 3.1, where error relative
to the *result* means nothing. The criterion is now error relative to the sum of
the term magnitudes, which is what bounds float32 accumulation regardless of
summation order. Both paths pass it.
Tests cover every remainder modulo eight, because the tail is handled in Go and
that boundary is where an off-by-one lives; cos(a,a)=1 exactly, which a dropped
lane passes on random data and fails here; and dot(ones,ones)=n, which says
outright whether every lane counts. Mutation-checked: a horizontal sum that
skips the upper half of the register reports "dot(ones,ones) = 4, want 8".
No AVX-512 path, though the machine measured here has one. It lowers clocks on
many parts and the gain on now-memory-bound code can be negative. That is a
separate measurement, not an addendum to this one.
go test ./... ok, 32 packages
go test -race -count=2 ./internal/vector/ ok
go test -tags noasm ./internal/vector/ ok
go vet, 7 targets + noasm/nosme on amd64/arm64 all pass
golangci-lint 0 issues
gosec 7, all in generated proto
…ently
The AVX2 work broke the gosec gate, and not by finding anything — it reported
zero issues. gosec's loader reads .go files and knows nothing about .s ones, so
the three functions internal/vector implements in assembly look like this:
internal/vector/vector_math_amd64.go:46:6: missing function body
The package then fails to type-check and is dropped from the analysis. The
gating pass says so and exits non-zero; the SARIF pass, which runs with
-no-fail, says so into a log nobody reads and uploads results with a hole in
them.
-tags noasm builds the scalar implementation in place of the assembly stubs, so
the package type-checks and stays under analysis. Nothing is hidden by it: the
assembly contains no syscalls, no paths, no allocation and no crypto — nothing
any gosec rule is looking for.
`make sec` had the same hole and was quieter about it: -quiet suppresses the
load errors, and gosec exits zero when it finds nothing in what it did manage
to read. It was reporting success over a package it had skipped. With the tag
it now scans 17 files and 5,279 lines there, and still finds nothing — which is
the same answer, this time actually asked.
Version set in the thirteen sources check-version.sh guards, and the CHANGELOG promoted from [Unreleased] to [2.13.0]. Adds scripts/bump-version.sh, which is what set them. The guard has known where the version lives in thirteen files for some time and says when they disagree, but nothing ever set them — a release meant thirteen hand edits and a re-run of the guard to find the one that was missed. That is the same shape as the bug this release fixed in the Makefile, where `make version` printed 2.11.4 through two releases because it held a fourteenth copy nothing checked. Its anchors are deliberately the same line shapes check-version.sh reads, so a number appearing elsewhere in a file — a changelog entry, a docs example, a dependency pin — is not touched: fourteen files changed, fourteen lines. It has no test suite of its own, matching how the repository treats its other tools rather than its guards; check-version.sh runs immediately after and is a stronger check than synthetic fixtures would be. What is in 2.13.0: Native Windows support, as far as "the tests pass there" goes and no further. WIN-001 through WIN-004: platform-split syscalls, an upstream fix to coder/hnsw so the vector index compiles at all, file-replacement semantics, and a windows-latest CI job that found nine failures on its first run. Two data-loss bugs found while auditing for that port, neither of them on any list: restore via the MCP tool and the replication snapshot install each had their own broken copy of a contract that had been fixed for HTTP and gRPC one release earlier. Its rollback had never executed since the day it was written. AVX2 vector maths on amd64, 3.7-6.3x on the search path, replacing a scalar loop that was the production path for the most common server architecture. A measured answer to a memory question inherited from a downstream fork: no leak, and the reasoning is in docs/DEPLOYMENT.md rather than a ticket.
spagu
changed the title
… the locks
CI went red on the release commit, in the two Python jobs, with:
error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided.
A uv lockfile records its own project's version, so bumping pyproject.toml
invalidates it. Both locks are regenerated with the uv version .github pins —
0.12.4 — rather than edited, because uv is the only thing that knows the format
it will accept, and an older uv would have rewritten them backwards. Three lines
changed across the two files; no dependency moved.
The more useful part is what the failure exposed. integrations/langchain-mddb
sits at the server's version and wraps mddb-client, so it speaks the server's
protocol — which is the stated reason every client and extension is on the
version guard's list. It was on neither that list nor the "has its own version"
list in VERSIONING.md. Nothing checked it, so nothing said it had drifted; the
bump announced the gap by breaking a lockfile. It is guarded now, along with the
`mddb-client>=` floor it declares: fifteen sources, not thirteen.
The lockfiles themselves stay off the guard on purpose. They are derived, and
`uv sync --locked` already refuses a lock that disagrees with its pyproject.
bump-version.sh regenerates them when uv is present and says exactly what to run
when it is not.
Also fixes two bugs in bump-version.sh, both mine, both from the same commit:
* the anchors were generated with unescaped inner quotes, so every pattern
containing a `"` closed the shell string early. Twelve of fifteen anchors
silently matched nothing, and the one containing `>=` was read as a
redirect — it created files called `=2.13.0]` in the repository root rather
than matching a line.
* bumping to the version already set reported "no line matched — check the
anchor". Re-running a bump is ordinary and it was sending the reader to look
for a broken anchor that was not there. The anchor and the replacement are
now separate arguments, so "matched nothing" and "already there" are
distinguishable rather than both being an unchanged file.
The Windows job went red on a run that took 359 seconds instead of the usual
230, which is exactly when a fixed sleep stops being long enough:
audit_test.go:52: want 3 events, got 0
audit_test.go:272: want 200, got 64
waitFlush slept 700 ms and returned. That is a guess about how long a batch
takes to reach the database, and the tests assert on the batch — so they now
wait for the batch. TEST-004 made this change across internal/audit and never
reached this file, which lives in the root package; this is the eighth instance
of the pattern in this release and the first found by a slow machine rather than
by reading.
Polling helps at both ends. Where the flush has already landed the call returns
at once instead of sleeping out the remainder of the guess: the audit tests go
from 4.9 seconds of pure waiting to 3.6 seconds in total.
go test -race -count=5 -run TestAudit ok, 19s
#211 — a fresh named volume restart-looped on first start. The image creates /app/data; this repository's own docker-compose.yml mounts a named volume at /data and points MDDB_PATH there. Docker copies an image directory's contents *and its ownership* into a fresh named volume only where that path exists in the image. /data did not, so it was created root-owned, the server runs as uid 1000, and the first start died with bolt.Open: open /data/mddb.db: permission denied Reproduced before fixing and measured rather than assumed: /data in a fresh volume was uid 0, /app/data in the image was uid 1000. The image now creates /data owned by the runtime user, and the container comes up healthy with the database created. Creating the directory was chosen over moving the mount to /app/data, which would have relocated the data of anyone already running. #212 — an API key sent as Authorization: Bearer was refused. It was parsed as a JWT and answered 401 {"error":"invalid token"} — a message about the credential when the problem was the header. The MCP middleware next door has always accepted the key in either place, so two surfaces of one server disagreed and clients meet the stricter one first. Keys carry the mddb_live_ prefix, now a named constant shared by the generator and the middleware, and it decides outright: nothing is validated twice and a JWT cannot be mistaken for a key. Mutation-checked — removing the branch reproduces the issue's exact 401. #213 — deleting a user only disabled it, and the name never came back. DELETE answered {"status":"deleted"} while the account stayed, marked disabled, holding the name against every later registration with 409. Delete-then-register is how a tenant's credentials get rotated, and it could not work for any name that had ever been used. Keeping the soft delete and letting register re-enable the account would have been the smaller change and the more dangerous one: the record carries the user's permissions and group memberships, so a name that looked free would have handed whoever claimed it the privileges of whoever held it before. The delete is now real and clears all four places a user is referenced — the record, its API keys, its per-collection permissions, its group memberships — in one transaction, because a half-done delete leaves a user with no record and the credentials that still let them in. I wrote the bug I was guarding against while writing it: groups are keyed by bare name, not the "group|" prefix users and permissions use, so the first version would have found no group and silently left the membership — precisely the inheritance path that ruled out the soft delete. Caught by reading how the rest of the code writes them. Four mutations, one per cleanup path, are all caught by the same subtest: the one that reloads from the database. The in-memory assertions passed under every one of them, because clearing the caches still ran. That is the shape of a fix that lasts until the next restart, and it is why the test checks both ends. This is a behaviour change and is documented as one — CHANGELOG under Changed, a "Changed in 2.13.0" block in docs/AUTHENTICATION.md, and the OpenAPI description. The audit log is untouched: that is where the record of who existed and who removed them belongs. go test ./... ok, 32 packages go test -race -count=3 (auth, delete) ok, 130s go vet ./... incl. tests, 7 targets all pass gofmt -s / golangci-lint clean, 0 issues docker build + fresh volume healthy, /data uid 1000
…mory question "What's New in MDDB 2.13" covers what changes for someone deciding whether to upgrade: the Windows job and what it is and is not, the AVX2 numbers, the two restore bugs, the three field fixes, and the one breaking change — user deletion is now a deletion. "Flat Throughput Is a Diagnosis" is the engineering half of the AVX2 work. Two sound optimisations delivered 1% and 1.5% against a predicted 33%, and the benchmark had been explaining why the whole time: identical throughput at 3 MB and 293 MB of working set means the memory system was never the constraint. The post also covers the correctness test that was wrong before the code was — the vectorised kernel proved more accurate than the scalar loop it had to match, because eight accumulators each carry an eighth of the rounding error. "Is It Leaking? Read a Different Number" answers the question a downstream fork raised and explains why resident memory cannot answer it. Their observation was probably accurate; the conclusion was not available from the figure they had. Every number and command is measured rather than recalled, per blog/README.md. The system/info response was taken from a running server and its values are the final sample of the 45-minute run the post describes. The `make soak` invocation was executed as printed. The test count in the release post was corrected from a guess to the 3,291 functions actually in the tree. make docs-build passes; all three render and cross-link.
|
spagu
marked this pull request as ready for review
August 24, 2026 19:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this release is
Native Windows support, in the narrow sense that the test suite passes there — and no further. The full suite runs on
windows-lateston every push. There are no Windows release artifacts; build from source withmake build-windows. WSL2 remains the supported route.AVX2 vector maths on amd64, replacing a scalar loop that was the production path for the most common server architecture.
Two data-loss bugs, neither of them planned work. They surfaced while auditing file-replacement semantics for the Windows port.
The Windows port
coder/hnswdid not compile for Windows: itsSavedGraph.Saveusesgoogle/renameio, which has no Windows build. Go compiles a package as a whole, so this blocked the vector index even though MDDB never callsSave. Fixed upstream — coder/hnsw#24 — with areplaceonto a fork as a bridge until it merges.diskSpaceoverstatfs/GetDiskFreeSpaceEx,processCPUTimeovergetrusage/GetProcessTimes. 100% statement coverage;make build-windowsproducesmddbd.exeandmddb-cli.exe. amd64 only.os.Renamedoes replace an existing file on Windows — it compiles toMoveFileExwithMOVEFILE_REPLACE_EXISTING, verified in the Go 1.27 source — so the plannedreplaceFilehelper was not written. What is real: unix domain sockets are now refused on Windows, becauseos.Chmodthere cannot express the owner-only mode bits the listener's security model rests on.windows-latestjob. Green on three consecutive runs.What the Windows job found on its first run
Nine failures, 31 of 32 packages passing. No bugs in MDDB's production code — but seven real bugs in tests and helpers, and six of those were present on Linux too. Windows did not break anything; it stopped forgiving.
newTestServerwas worse: it ignored the error from its ownos.Removeand leaked a database per test in silence.windows.Filetime.Nanoseconds()on a duration does not read as "roughly minus 369 years". The epoch subtraction reaches -1.16e19, an int64 stops at -9.22e18, and the multiply wraps to6802270474709551616— a large positive value that looks like a reading rather than an error.os.Renamereplaces atomically, but not under concurrency. Windows must delete the target first and deletion there is not immediate, so a rename landing in that window fails withAccess is denied.SEC-017 and SEC-018 — not on any list
restore_backup.godescribes itself as "the single implementation behind POST /v1/restore and the gRPC Restore RPC". It was, for those two. Two more entry points had their own copies:restoretool: no validation, no restore lock, no rollback, no cache rebuild. Any readable file inside the backup directory overwrote the live database. It did not have to be a database: the copy succeeds on bytes and only the reopen fails, by which point the original is gone. Measured before the fix:write after a failed MCP restore failed: database not open. The read after the failure still succeeded, served from cache — the dead database looked healthy until the first write.All three callers now share one contract,
swapDatabase(source, install).The rollback had never run. Coverage put its entire closure and both call sites at zero executions: every existing failure test supplied a source that validation rejected, so the swap returned before touching the live file. The code whose only job is to save the database when a restore fails had not executed since it was written for SEC-015.
SRCH-011 — AVX2
Measured with the benchmark the original observation used, dims=768, median of five runs:
The shape matters as much as the multiplier. The scalar path was flat — the same 6.8 GB/s whether the working set fit in cache or was fifteen times larger. Memory-bound code does not do that; it was bound by instruction throughput. That explains both earlier attempts to speed it up, a batch kernel measured within noise and a hoisted query norm that predicted 33% and delivered 1.5%. Both cut arithmetic over the same bytes.
Go assembly rather than cgo, decided by one line in
release.yml: releases build withCGO_ENABLED=0, so a cgo kernel would compile locally and never ship. Selection is a runtime CPU check — there is one amd64 build of MDDB and it must start on a machine without AVX2.On correctness: the ticket asked for agreement within 1e-5, I wrote that test, and AVX2 failed two cases. Running the same test against the scalar build failed four. The kernel is more accurate than the loop it replaces — eight accumulators each carry an eighth of the rounding error — so the test was wrong, not the code. The criterion is now error relative to the sum of term magnitudes, which is what bounds float32 accumulation regardless of summation order.
GO-041 — the memory question, answered
A downstream fork reported RSS going from 42 MB to 153 MB under load. Measured here over 45 minutes, 1,261,570 operations, zero errors, against a database that grew to 160 MB:
Three heap profiles rather than two, because a start-to-end diff cannot distinguish "grew early then settled" from "grew throughout". The largest allocator gained 6.7 MB in the first half and lost 2.6 MB in the second. No leak. RSS includes bbolt's mmap of the database file, which is file-backed and reclaimable.
docs/DEPLOYMENT.mdnow says to watchmemoryHeapand the goroutine count instead.Also in here
tools/bench/soak— the sustained-load harness that produced those numbers.scripts/check-source-encoding.sh— PowerShell's>writes UTF-16 LE with a BOM, and Go answers such a file with "illegal UTF-8 encoding", naming no cause.scripts/bump-version.sh— the version guard has known where the version lives in fifteen files for some time and said when they disagreed, but nothing ever set them.make buildwas documented in the README and did not exist.make versionprinted 2.11.4 through two releases, because it held a copy nothing checked.internal/vectoronce it contained assembly: a declaration without a body is a type error to its loader, and-quiethid that it had read nothing.Not in here
No Windows release artifacts.
DOC-018(a missing document answers 400 rather than 404) is filed but unfixed — it breaks any client keying on 400 and needs a decision.WIN-005removes the hnswreplaceonce upstream merges.