Skip to content

perf(users): rebuild user index with one shared scan and a bulk write - #638

Open
MorquinDevlar wants to merge 1 commit into
GoMudEngine:masterfrom
MorquinDevlar:morq-fast-index-rebuild
Open

perf(users): rebuild user index with one shared scan and a bulk write#638
MorquinDevlar wants to merge 1 commit into
GoMudEngine:masterfrom
MorquinDevlar:morq-fast-index-rebuild

Conversation

@MorquinDevlar

Copy link
Copy Markdown
Contributor

What this does

Startup currently pays two full passes over every user file:

  1. The user index rebuild unmarshals each file into a complete UserRecord, then appends records to users.idx one at a time - each AddUser call opens the file, appends, rewrites the header, and fsyncs. The existing BenchmarkAddUser clocks that at ~3.7ms per record on a fast SSD.
  2. CharacterIndex.Rebuild() runs a second full unmarshal of every user file on every boot, including copyovers, just to extract active character names.

This PR feeds both indexes from one lightweight scan that decodes only userid, username, and the active character name, and writes the user index in one atomic pass (temp file + rename, single fsync) with the directory checksum folded into the same write. The on-disk index format is unchanged, and the IsUpToDate checksum skip works exactly as before.

At 1000 users (~5-8KB files, warm cache) a boot-after-activity rebuild goes from roughly 4 seconds to roughly 0.12 seconds.

Also hardened

  • A truncated index (crash mid-AddUser) with an intact header could previously pass the checksum test while loadRecords silently failed, booting with empty maps - at which point GetUniqueUserId starts handing out userids that already belong to existing user files, and the next registration overwrites one of them. loadRecords now logs its failures, and IsUpToDate refuses to trust an index whose loaded record count disagrees with its header.
  • One unreadable or malformed user file previously aborted the entire rebuild walk silently, truncating the index. The scan now skips such files with a warning.
  • The scan warns about anomalies that usually mean hand-edited data: duplicate userids, duplicate usernames, and numeric filenames that disagree with the userid inside the file.

Compatibility

  • On-disk index format unchanged (version 2).
  • Old-format username.yaml files are still indexed - the scan reads content, not filenames.
  • SearchOfflineUsers is untouched; UserIndex.Rebuild() and CharacterIndex.Rebuild() keep their signatures.

Numbers (Apple M5 Pro, warm cache)

current this PR
Parse per file (stock admin user file) 123µs (full UserRecord) x2 passes 90µs x1 pass
Index write, 1000 users ~3.7s (fsync per record) ~5ms (one write)
Boot after activity, 1000 users ~4s ~0.12s

Reproduce with:

go test -bench 'BenchmarkRebuildFromScan|BenchmarkScanVsFullUnmarshal|BenchmarkAddUser' -run '^$' ./internal/users/

BenchmarkScanRealUsers also accepts BENCH_USERS_DIR=/path/to/users to measure against a real users directory (read-only - the index it writes goes to a temp dir).

Tests

  • Scan correctness: valid files, old-format files, alt/malformed/id-less files skipped.
  • Rebuild round-trip: records, lookup maps, highest userid, and checksum survive a write/reload cycle.
  • Stale entries do not survive a rebuild.
  • A truncated index is never reported up to date.

Startup previously paid two full passes over every user file: the user
index rebuild unmarshaled each file into a complete UserRecord and then
appended records one at a time (an fsync per user, ~3.7ms each), and
CharacterIndex.Rebuild ran a second full unmarshal of every file on every
boot, including copyovers.

Both indexes are now fed by a single lightweight scan that decodes only
userid, username, and the active character name, and the user index is
written in one atomic pass (temp file + rename, single fsync) with the
directory checksum folded into the same write. At 1000 users this takes a
boot-after-activity rebuild from roughly 4s to roughly 0.1s.

The scan also hardens rebuild behavior: unreadable or malformed user files
are skipped with a warning instead of silently aborting the walk, and
anomalies that usually mean hand-edited data (duplicate userids, duplicate
usernames, a filename that disagrees with the userid inside) are logged.

Also fixes a silent-failure edge in index loading: loadRecords now logs
read failures, and IsUpToDate refuses to trust an index whose records
section is shorter than its header claims. Previously a truncated index
with an intact header could pass the checksum test and boot with empty
maps, which would make GetUniqueUserId hand out userids that already
belong to existing user files.
@MorquinDevlar
MorquinDevlar requested a review from Volte6 as a code owner July 27, 2026 06:55
pruuk added a commit to pruuk/DOGMud that referenced this pull request Jul 28, 2026
… write

Port of upstream GoMud PR GoMudEngine#638 (MorquinDevlar). The old Rebuild fully
YAML-decoded every user via SearchOfflineUsers and appended records
one-at-a-time (a file open + header rewrite per user). Worse, a single
malformed user file returned an error from the walk callback and ABORTED
the scan — every user after the bad file silently vanished from the
index and could no longer log in.

Rebuild now walks the users dir once, decodes only userid/username into
a two-field struct, collects records in memory, and writes header +
records to a temp file renamed over users.idx (a crash mid-write can
never leave a truncated index). Malformed/unreadable files skip with a
warning; duplicate userids/usernames skip (first wins) and are logged;
filename/content userid mismatches are logged. On-disk V1 format is
unchanged.

Upstream GoMudEngine#639 (mtime/size incremental sync, index v3) deliberately NOT
ported: it is still a draft upstream, our copyover path already skips
the rebuild entirely (state rides the pipe), and at ~53 prod users the
single minimal scan is already sub-millisecond. Revisit at scale.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pruuk added a commit to pruuk/DOGMud that referenced this pull request Jul 28, 2026
pruuk added a commit to pruuk/DOGMud that referenced this pull request Jul 28, 2026
The boot-time mob/player name-collision audit called
users.CharacterNameSearch once per mob template, and each call walks the
whole users directory FULLY YAML-decoding every user file plus reading
every .alts.yaml. At 641 mob names x every user file that was ~34k
decodes — measured as a 21s silent boot gap locally and 46s on the prod
droplet, where it was nearly the ENTIRE copyover pause (conversations.
Load -> mutators: 46s of a ~50s window, on both cold and copyover
boots).

users.CharacterNameIndex() now builds a case-folded name -> userId map
(mains + alts) in one minimal-decode pass — same hardening as the GoMudEngine#638
index scan (malformed files skip with a warning). The audit checks each
mob name against the map: 32ms at 98 characters locally. Expected prod
copyover pause: ~50s -> a few seconds.

CharacterNameSearch is unchanged for one-off lookups (mail recipient
resolution etc.).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant