From 0fa5bd0d71685556e712cde78002cfd44ee4c2b9 Mon Sep 17 00:00:00 2001 From: MorquinDevlar Date: Mon, 27 Jul 2026 08:26:24 +0200 Subject: [PATCH 1/2] perf(users): rebuild user index with one shared scan and a bulk write 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. --- internal/users/charindex.go | 20 +- internal/users/charindex_test.go | 10 +- internal/users/index.go | 115 +++++++----- internal/users/indexscan.go | 110 +++++++++++ internal/users/indexscan_test.go | 302 +++++++++++++++++++++++++++++++ main.go | 24 ++- 6 files changed, 518 insertions(+), 63 deletions(-) create mode 100644 internal/users/indexscan.go create mode 100644 internal/users/indexscan_test.go diff --git a/internal/users/charindex.go b/internal/users/charindex.go index d54ac0478..2d77daa6f 100644 --- a/internal/users/charindex.go +++ b/internal/users/charindex.go @@ -69,19 +69,25 @@ func (ci *CharacterIndex) Find(name string) (userId int, found bool) { return } -// Rebuild clears the index and repopulates it from every user record on disk +// Rebuild clears the index and repopulates it from every user file on disk // plus all currently online users. Only active character names are added here; // the alt-characters module is responsible for adding alt names after this // runs. func (ci *CharacterIndex) Rebuild() { - newMap := make(map[string]int) + ci.RebuildFromScan(ScanUserFiles()) +} - SearchOfflineUsers(func(u *UserRecord) bool { - if u.Character != nil && u.Character.Name != "" { - newMap[strings.ToLower(u.Character.Name)] = u.UserId +// RebuildFromScan is Rebuild fed by an existing user file scan, so startup +// can share one scan between the user index and the character index instead +// of fully parsing every user record a second time. +func (ci *CharacterIndex) RebuildFromScan(scan []UserFileScan) { + newMap := make(map[string]int, len(scan)) + + for _, s := range scan { + if s.CharacterName != "" { + newMap[strings.ToLower(s.CharacterName)] = s.UserId } - return true - }) + } for _, u := range GetAllActiveUsers() { if u.Character != nil && u.Character.Name != "" { diff --git a/internal/users/charindex_test.go b/internal/users/charindex_test.go index 8a9a74721..e5fd411b5 100644 --- a/internal/users/charindex_test.go +++ b/internal/users/charindex_test.go @@ -4,6 +4,8 @@ import ( "fmt" "sync" "testing" + + "github.com/GoMudEngine/GoMud/internal/mudlog" ) func freshCharacterIndex() *CharacterIndex { @@ -116,8 +118,12 @@ func TestCharacterIndex_MultipleUsersMultipleNames(t *testing.T) { } func TestCharacterIndex_Rebuild(t *testing.T) { + // The scan warns about the missing users directory in a test env, so the + // logger must be initialized. + mudlog.SetupLogger(nil, "", "", false) + // Swap in a fresh singleton so Rebuild exercises the real code path - // without touching disk (SearchOfflineUsers finds nothing in a test env). + // without touching disk (ScanUserFiles finds nothing in a test env). orig := characterIndex defer func() { characterIndex = orig }() @@ -127,7 +133,7 @@ func TestCharacterIndex_Rebuild(t *testing.T) { // Pre-populate with stale data that Rebuild should clear. ci.Add("stale", 99) - // Rebuild will call SearchOfflineUsers (returns nothing in test env) and + // Rebuild will call ScanUserFiles (returns nothing in test env) and // GetAllActiveUsers (returns nothing since userManager is empty). The stale // entry must be gone. ci.Rebuild() diff --git a/internal/users/index.go b/internal/users/index.go index 8546b5520..87c94192b 100644 --- a/internal/users/index.go +++ b/internal/users/index.go @@ -104,6 +104,7 @@ func (idx *UserIndex) loadRecords() { f, err := os.Open(idx.Filename) if err != nil { + mudlog.Error("UserIndex", "error", "failed to open index file", "path", idx.Filename, "details", err) return } defer f.Close() @@ -111,9 +112,11 @@ func (idx *UserIndex) loadRecords() { dataSize := idx.metaData.RecordCount * idx.metaData.RecordSize buf := make([]byte, dataSize) if _, err := f.Seek(int64(idx.metaData.MetaDataSize), io.SeekStart); err != nil { + mudlog.Error("UserIndex", "error", "failed to seek past index header", "path", idx.Filename, "details", err) return } if _, err := io.ReadFull(f, buf); err != nil { + mudlog.Error("UserIndex", "error", "index file is truncated or corrupt, a rebuild will recreate it", "path", idx.Filename, "details", err) return } @@ -209,8 +212,16 @@ func computeDirChecksum(basePath string) (uint64, error) { } // IsUpToDate returns true if the index file exists, has the current version, -// and its stored FNV-64 checksum matches the current state of the user directory. +// actually loaded every record its header claims, and its stored FNV-64 +// checksum matches the current state of the user directory. func (idx *UserIndex) IsUpToDate() bool { + basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + return idx.isUpToDateForDir(basePath) +} + +// isUpToDateForDir is IsUpToDate parameterized by the directory to compare +// against, so tests can point it at a synthetic users directory. +func (idx *UserIndex) isUpToDateForDir(basePath string) bool { if !idx.Exists() { return false } @@ -218,7 +229,14 @@ func (idx *UserIndex) IsUpToDate() bool { return false } - basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + // A truncated or unreadable records section leaves fewer records in + // memory than the header claims. Such an index must never be trusted: + // with empty maps, GetUniqueUserId would start handing out userids that + // already belong to existing user files. + if uint64(len(idx.records)) != idx.metaData.RecordCount { + return false + } + current, err := computeDirChecksum(basePath) if err != nil { return false @@ -226,68 +244,70 @@ func (idx *UserIndex) IsUpToDate() bool { return idx.metaData.Checksum == current } -// Rebuild recreates the index from all offline user records. -// It calls Create() internally so it is self-contained. -// After building, it computes and persists a directory checksum so that -// IsUpToDate can detect stale indexes on the next startup. +// Rebuild recreates the index from the user files on disk. It runs a +// lightweight scan (userid and username only) instead of fully loading every +// user record, then writes the whole index in one atomic pass (temp file + +// rename) with a single sync, instead of appending and syncing once per +// user. The directory checksum is folded into that same write so IsUpToDate +// can detect stale indexes on the next startup. func (idx *UserIndex) Rebuild() error { - if err := idx.Create(); err != nil { - return fmt.Errorf("index create failed: %w", err) - } - - var firstErr error - SearchOfflineUsers(func(u *UserRecord) bool { - if err := idx.AddUser(u.UserId, u.Username); err != nil { - mudlog.Error("UserIndex.Rebuild", "error", err.Error(), "userId", u.UserId, "username", u.Username) - if firstErr == nil { - firstErr = err - } - } - return true - }) - - if firstErr != nil { - return firstErr - } + return idx.RebuildFromScan(ScanUserFiles()) +} +// RebuildFromScan is Rebuild fed by an existing scan, so startup can share +// one scan between the user index and the character index. +func (idx *UserIndex) RebuildFromScan(scan []UserFileScan) error { basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) checksum, err := computeDirChecksum(basePath) if err != nil { return fmt.Errorf("checksum compute failed: %w", err) } - if err := idx.writeChecksum(checksum); err != nil { - return fmt.Errorf("checksum write failed: %w", err) - } - - return nil + return idx.applyScan(scan, checksum) } -// writeChecksum persists a new checksum value into the index header on disk -// and updates the in-memory metadata. -func (idx *UserIndex) writeChecksum(checksum uint64) error { +// applyScan replaces the in-memory records and lookup maps with the scan +// results, then writes the complete index to disk once. +func (idx *UserIndex) applyScan(scan []UserFileScan, checksum uint64) error { + + records := make([]IndexUserRecord, 0, len(scan)) + for _, s := range scan { + rec := IndexUserRecord{UserID: int64(s.UserId)} + copy(rec.Username[:], strings.ToLower(s.Username)) + records = append(records, rec) + } + idx.mu.Lock() defer idx.mu.Unlock() - idx.metaData.Checksum = checksum - - headerBytes, err := idx.metaData.Format() - if err != nil { - return err + idx.metaData = IndexMetaData{ + MetaDataSize: FixedHeaderTotalLength, + IndexVersion: IndexVersion, + RecordCount: uint64(len(records)), + RecordSize: IndexRecordSizeV1, + Checksum: checksum, } - f, err := os.OpenFile(idx.Filename, os.O_RDWR, 0644) - if err != nil { - return err - } - defer f.Close() + idx.records = records + idx.byUsername = make(map[string]int64, len(records)) + idx.byUserId = make(map[int64]string, len(records)) + idx.highestUserId = 0 - if _, err := f.Seek(0, io.SeekStart); err != nil { - return err + for _, rec := range records { + username := string(bytes.TrimRight(rec.Username[:], "\x00")) + idx.byUsername[username] = rec.UserID + idx.byUserId[rec.UserID] = username + if int(rec.UserID) > idx.highestUserId { + idx.highestUserId = int(rec.UserID) + } } - if _, err := f.Write(headerBytes); err != nil { - return err + + // The in-memory state is updated even if the disk write fails - the + // running process must trust what was just scanned, not a stale file. + if err := idx.writeCompleteIndex(records); err != nil { + return fmt.Errorf("index write failed: %w", err) } - return f.Sync() + + return nil } func (idx *UserIndex) GetMetaData() IndexMetaData { @@ -360,6 +380,7 @@ func (idx *UserIndex) getMetaDataFromFile() IndexMetaData { headerContent := strings.TrimSpace(string(header[:FixedHeaderTotalLength-1])) n, _ := fmt.Sscanf(headerContent, "VERSION=%d,RECORDCOUNT=%d,RECORDSIZE=%d,CHECKSUM=%d", &meta.IndexVersion, &meta.RecordCount, &meta.RecordSize, &meta.Checksum) if n < 3 { + mudlog.Error("UserIndex", "error", "index header is unparseable, a rebuild will recreate it", "path", idx.Filename) return IndexMetaData{} } diff --git a/internal/users/indexscan.go b/internal/users/indexscan.go new file mode 100644 index 000000000..76aa77e16 --- /dev/null +++ b/internal/users/indexscan.go @@ -0,0 +1,110 @@ +package users + +import ( + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/GoMudEngine/GoMud/internal/configs" + "github.com/GoMudEngine/GoMud/internal/mudlog" + "github.com/GoMudEngine/GoMud/internal/util" + "gopkg.in/yaml.v2" +) + +// UserFileScan holds the fields a lightweight pass over a user file yields. +// The user index and the character index are both built from these at +// startup, without paying for a full UserRecord unmarshal per file. +type UserFileScan struct { + UserId int + Username string + CharacterName string +} + +// userFileScanFields is the minimal unmarshal target for a scan. The file +// is still lexed in full, but decoding into this instead of a full +// UserRecord is measurably cheaper and builds no throwaway record - see +// BenchmarkScanVsFullUnmarshal. +type userFileScanFields struct { + UserId int `yaml:"userid"` + Username string `yaml:"username"` + Character struct { + Name string `yaml:"name"` + } `yaml:"character"` +} + +// ScanUserFiles runs a lightweight scan over the configured users directory. +func ScanUserFiles() []UserFileScan { + basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + return scanUserFilesInDir(basePath) +} + +// scanUserFilesInDir reads the userid, username, and active character name +// of every user file under basePath. Alt files are never opened. Files that +// cannot be read or parsed are skipped with a warning instead of aborting +// the scan, and anomalies that usually mean hand-edited data (duplicate +// userids, duplicate usernames, a numeric filename that disagrees with the +// userid inside the file) are logged so they get noticed. +func scanUserFilesInDir(basePath string) []UserFileScan { + + results := []UserFileScan{} + seenIds := make(map[int]string) + seenNames := make(map[string]string) + + filepath.Walk(basePath, func(path string, info os.FileInfo, err error) error { + + if err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "walk_error", err) + return nil + } + + if info.IsDir() { + return nil + } + + if !strings.HasSuffix(path, `.yaml`) || strings.HasSuffix(path, `.alts.yaml`) { + return nil + } + + fileBytes, err := os.ReadFile(path) + if err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "read_error", err) + return nil + } + + var scanned userFileScanFields + if err := yaml.Unmarshal(fileBytes, &scanned); err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "unmarshal_error", err) + return nil + } + + if scanned.UserId < 1 || scanned.Username == `` { + mudlog.Warn("ScanUserFiles", "info", "skipping user file missing userid or username", "path", path) + return nil + } + + if fileId, convErr := strconv.Atoi(strings.TrimSuffix(filepath.Base(path), `.yaml`)); convErr == nil && fileId != scanned.UserId { + mudlog.Warn("ScanUserFiles", "info", "filename does not match userid in file", "path", path, "userid", scanned.UserId) + } + + if otherPath, ok := seenIds[scanned.UserId]; ok { + mudlog.Warn("ScanUserFiles", "info", "duplicate userid", "userid", scanned.UserId, "path", path, "otherpath", otherPath) + } + lowerName := strings.ToLower(scanned.Username) + if otherPath, ok := seenNames[lowerName]; ok { + mudlog.Warn("ScanUserFiles", "info", "duplicate username", "username", scanned.Username, "path", path, "otherpath", otherPath) + } + seenIds[scanned.UserId] = path + seenNames[lowerName] = path + + results = append(results, UserFileScan{ + UserId: scanned.UserId, + Username: scanned.Username, + CharacterName: scanned.Character.Name, + }) + + return nil + }) + + return results +} diff --git a/internal/users/indexscan_test.go b/internal/users/indexscan_test.go new file mode 100644 index 000000000..de7d0862d --- /dev/null +++ b/internal/users/indexscan_test.go @@ -0,0 +1,302 @@ +package users + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoMudEngine/GoMud/internal/mudlog" + "gopkg.in/yaml.v2" +) + +// writeScanTestUser writes a user file shaped like a real record: the index +// fields up top and an embedded character block padded out so the scan pays +// a realistic lexing cost. Production user files run 10-20KB; padKB controls +// how close a test file gets to that. +func writeScanTestUser(t testing.TB, dir string, userId int, username string, charName string, padKB int) { + t.Helper() + + var sb strings.Builder + fmt.Fprintf(&sb, "userid: %d\n", userId) + fmt.Fprintf(&sb, "username: %s\n", username) + sb.WriteString("password: 52c69134f0185daafe43fa511b6d1db16e59404a7992aa0d9bfa0bea05d592a9\n") + sb.WriteString("joined: 2026-01-07T13:52:18.427407+02:00\n") + if charName != `` { + fmt.Fprintf(&sb, "character:\n name: %s\n roomid: 1\n level: 5\n experience: 1234\n", charName) + for i := 0; sb.Len() < padKB*1024; i++ { + fmt.Fprintf(&sb, " itemfiller%d: some padding value that stands in for inventory and buffs %d\n", i, i) + } + } + + if err := os.WriteFile(filepath.Join(dir, fmt.Sprintf(`%d.yaml`, userId)), []byte(sb.String()), 0644); err != nil { + t.Fatal(err) + } +} + +func newScanTestIndex(dir string) *UserIndex { + return &UserIndex{ + Filename: filepath.Join(dir, `users.idx`), + byUsername: make(map[string]int64), + byUserId: make(map[int64]string), + } +} + +// TestScanUserFilesInDir verifies the scan picks up every valid user file, +// including old-format username.yaml files, while skipping alt files, +// malformed yaml, and files without a userid. +func TestScanUserFilesInDir(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 5; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + // A user file in the old username.yaml naming, still valid by content. + if err := os.WriteFile(filepath.Join(dir, `legacy.yaml`), []byte("userid: 77\nusername: legacy\ncharacter:\n name: Oldtimer\n"), 0644); err != nil { + t.Fatal(err) + } + + junkFiles := map[string]string{ + `3.alts.yaml`: "alts:\n- name: x\n", + `broken.yaml`: "userid: [not closed\n", + `50.yaml`: "username: idless\n", + `notes.txt`: `hello`, + } + for name, content := range junkFiles { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0644); err != nil { + t.Fatal(err) + } + } + + scan := scanUserFilesInDir(dir) + + if len(scan) != 6 { + t.Fatalf("expected 6 scanned users, got %d", len(scan)) + } + + byId := make(map[int]UserFileScan, len(scan)) + for _, s := range scan { + byId[s.UserId] = s + } + + if s, ok := byId[3]; !ok || s.Username != `User_3` || s.CharacterName != `Hero_3` { + t.Errorf("expected userId 3 with username 'User_3' and character 'Hero_3', got %+v", s) + } + if s, ok := byId[77]; !ok || s.Username != `legacy` || s.CharacterName != `Oldtimer` { + t.Errorf("expected old-format file to scan as userId 77 'legacy'/'Oldtimer', got %+v", s) + } +} + +// TestRebuildFromScanRoundTrip verifies applyScan builds correct lookup +// state, persists it atomically, and that a fresh UserIndex reads back the +// identical records and checksum. +func TestRebuildFromScanRoundTrip(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 10; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + checksum, err := computeDirChecksum(dir) + if err != nil { + t.Fatal(err) + } + + idx := newScanTestIndex(dir) + if err := idx.applyScan(scanUserFilesInDir(dir), checksum); err != nil { + t.Fatalf("applyScan failed: %v", err) + } + + if userId, found := idx.FindByUsername(`user_7`); !found || userId != 7 { + t.Errorf("expected to find 'user_7' with userId 7, got %d, found=%v", userId, found) + } + if highest := idx.GetHighestUserId(); highest != 10 { + t.Errorf("expected highest userId 10, got %d", highest) + } + + reloaded := newScanTestIndex(dir) + reloaded.metaData = reloaded.getMetaDataFromFile() + reloaded.loadRecords() + + if reloaded.metaData.RecordCount != 10 { + t.Fatalf("expected 10 records after reload, got %d", reloaded.metaData.RecordCount) + } + if reloaded.metaData.Checksum != checksum { + t.Errorf("expected persisted checksum %d, got %d", checksum, reloaded.metaData.Checksum) + } + for i := 1; i <= 10; i++ { + username, found := reloaded.FindByUserId(i) + if !found || username != fmt.Sprintf(`user_%d`, i) { + t.Errorf("expected 'user_%d' for userId %d, got '%s', found=%v", i, i, username, found) + } + } +} + +// TestRebuildFromScanReplacesStaleIndex verifies index entries with no +// matching user file do not survive a rebuild. +func TestRebuildFromScanReplacesStaleIndex(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 5; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), ``, 0) + } + + idx := newScanTestIndex(dir) + if err := idx.Create(); err != nil { + t.Fatal(err) + } + if err := idx.AddUser(999, `phantom`); err != nil { + t.Fatal(err) + } + + checksum, err := computeDirChecksum(dir) + if err != nil { + t.Fatal(err) + } + if err := idx.applyScan(scanUserFilesInDir(dir), checksum); err != nil { + t.Fatalf("applyScan failed: %v", err) + } + + if _, found := idx.FindByUsername(`phantom`); found { + t.Error("phantom user survived rebuild") + } + if highest := idx.GetHighestUserId(); highest != 5 { + t.Errorf("expected highest userId 5 after rebuild, got %d", highest) + } +} + +// TestIsUpToDateRejectsTruncatedIndex verifies the header/records +// consistency guard: an index whose records section was cut short must +// never report up to date, even when the directory checksum still matches. +func TestIsUpToDateRejectsTruncatedIndex(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + usersDir := t.TempDir() + + for i := 1; i <= 5; i++ { + writeScanTestUser(t, usersDir, i, fmt.Sprintf(`User_%d`, i), ``, 0) + } + + checksum, err := computeDirChecksum(usersDir) + if err != nil { + t.Fatal(err) + } + + idx := newScanTestIndex(usersDir) + if err := idx.applyScan(scanUserFilesInDir(usersDir), checksum); err != nil { + t.Fatalf("applyScan failed: %v", err) + } + if !idx.isUpToDateForDir(usersDir) { + t.Fatal("expected freshly rebuilt index to be up to date") + } + + // Cut the records section short, simulating a crash mid-write, and + // reload the way startup does. + fileBytes, err := os.ReadFile(idx.Filename) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(idx.Filename, fileBytes[:len(fileBytes)-20], 0644); err != nil { + t.Fatal(err) + } + + truncated := newScanTestIndex(usersDir) + truncated.metaData = truncated.getMetaDataFromFile() + truncated.loadRecords() + + if truncated.isUpToDateForDir(usersDir) { + t.Fatal("truncated index must not report up to date") + } +} + +// BenchmarkRebuildFromScan measures scan plus rebuild over synthetic user +// directories with realistically sized files (embedded character block). +func BenchmarkRebuildFromScan(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + for _, userCt := range []int{100, 1000, 10000} { + b.Run(fmt.Sprintf(`%d_users`, userCt), func(b *testing.B) { + dir := b.TempDir() + for i := 1; i <= userCt; i++ { + writeScanTestUser(b, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 8) + } + idx := newScanTestIndex(dir) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + checksum, err := computeDirChecksum(dir) + if err != nil { + b.Fatal(err) + } + if err := idx.applyScan(scanUserFilesInDir(dir), checksum); err != nil { + b.Fatal(err) + } + } + }) + } +} + +// BenchmarkScanVsFullUnmarshal isolates the parse cost the scan avoids: +// unmarshaling a real user file (the stock admin record with an embedded +// character) into the minimal scan struct versus a full UserRecord. +func BenchmarkScanVsFullUnmarshal(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + fileBytes, err := os.ReadFile(`../../_datafiles/world/default/users/1.yaml`) + if err != nil { + b.Skip(`stock user file not available`) + } + + b.Run(`minimal_scan`, func(b *testing.B) { + for i := 0; i < b.N; i++ { + var scanned userFileScanFields + if err := yaml.Unmarshal(fileBytes, &scanned); err != nil { + b.Fatal(err) + } + } + }) + + b.Run(`full_userrecord`, func(b *testing.B) { + for i := 0; i < b.N; i++ { + var u UserRecord + if err := yaml.Unmarshal(fileBytes, &u); err != nil { + b.Fatal(err) + } + } + }) +} + +// BenchmarkScanRealUsers scans a real users directory named by the +// BENCH_USERS_DIR environment variable. The directory is only read - the +// rebuilt index is written to the benchmark temp dir - so it is safe to +// point at a live server's users directory. Skipped when the variable is +// unset. +func BenchmarkScanRealUsers(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + usersDir := os.Getenv(`BENCH_USERS_DIR`) + if usersDir == `` { + b.Skip(`set BENCH_USERS_DIR to a users directory to run this benchmark`) + } + + idx := newScanTestIndex(b.TempDir()) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + checksum, err := computeDirChecksum(usersDir) + if err != nil { + b.Fatal(err) + } + if err := idx.applyScan(scanUserFilesInDir(usersDir), checksum); err != nil { + b.Fatal(err) + } + } + b.StopTimer() + + b.ReportMetric(float64(idx.GetMetaData().RecordCount), `users`) +} diff --git a/main.go b/main.go index 7e35190af..f5dd115c0 100644 --- a/main.go +++ b/main.go @@ -262,23 +262,33 @@ func main() { isCopyover := flags.CopyoverFd() >= 0 if !isCopyover { - timeStart := time.Now() idx := users.InitUserIndex() if !idx.Exists() { // Since it doesn't exist yet, that's a good indication we should do a quick format migration check users.DoUserMigrations() } + } + + // One lightweight scan of the user files (userid, username, character + // name) feeds both the user index and the character index, instead of + // each index fully parsing every user record on its own. + scanStart := time.Now() + userScan := users.ScanUserFiles() + + if !isCopyover { + idx := users.GetUserIndex() if idx.IsUpToDate() { - mudlog.Info("UserIndex", "info", "User index up to date.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(timeStart)) + mudlog.Info("UserIndex", "info", "User index up to date.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(scanStart)) } else { - idx.Create() - idx.Rebuild() - mudlog.Info("UserIndex", "info", "User index recreated.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(timeStart)) + if err := idx.RebuildFromScan(userScan); err != nil { + mudlog.Error("UserIndex", "error", "rebuild failed", "details", err) + } + mudlog.Info("UserIndex", "info", "User index recreated.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(scanStart)) } } - users.GetCharacterIndex().Rebuild() - mudlog.Info("CharacterIndex", "info", "Active character names indexed.", "characters", users.GetCharacterIndex().Len()) + users.GetCharacterIndex().RebuildFromScan(userScan) + mudlog.Info("CharacterIndex", "info", "Active character names indexed.", "characters", users.GetCharacterIndex().Len(), "time taken", time.Since(scanStart)) // Load the round count from the file if !isCopyover { From 2fd1d08888addfd34cf368b683f54f1f954df0f5 Mon Sep 17 00:00:00 2001 From: MorquinDevlar Date: Mon, 27 Jul 2026 08:32:59 +0200 Subject: [PATCH 2/2] perf(users): sync the user index incrementally instead of rescanning every file Widens the index record (version 3) to carry the active character name plus the mtime and size the user file had when it was indexed. Startup then syncs instead of rebuilding: one directory listing is compared against the stored records, unchanged files keep their entry without being opened, and only files that are new or changed since the last index write get parsed. Records whose files are gone are dropped. When nothing changed the index file is not rewritten at all. The character index is now rebuilt straight from the index records, so a boot where nothing changed - including every copyover - reads zero user files. Startup cost becomes proportional to how many users were active since the last restart instead of total accounts. Measured at 1000 users (8KB files, warm cache): a no-change sync takes 2.4ms and a 50-file churn sync 18ms, versus roughly 120ms for the full scan this replaces and roughly 4s for the original per-user rebuild. An index in the old format, or one that fails to load cleanly, falls back to one full scan and is rewritten in the new format - the first boot after upgrading pays one full scan, every boot after that syncs. Runtime AddUser writes a stub record (zero mtime), which the next sync re-parses and completes, preserving the register-then-save ordering. --- internal/users/charindex.go | 25 +++ internal/users/index.go | 191 ++++++++++++++++++-- internal/users/indexscan.go | 74 +++++--- internal/users/indexsync_test.go | 287 +++++++++++++++++++++++++++++++ main.go | 39 ++--- 5 files changed, 546 insertions(+), 70 deletions(-) create mode 100644 internal/users/indexsync_test.go diff --git a/internal/users/charindex.go b/internal/users/charindex.go index 2d77daa6f..a08cf17e3 100644 --- a/internal/users/charindex.go +++ b/internal/users/charindex.go @@ -1,6 +1,7 @@ package users import ( + "bytes" "strings" "sync" ) @@ -77,6 +78,30 @@ func (ci *CharacterIndex) Rebuild() { ci.RebuildFromScan(ScanUserFiles()) } +// RebuildFromIndex repopulates the character index straight from the user +// index records plus all currently online users, without opening a single +// user file. +func (ci *CharacterIndex) RebuildFromIndex(idx *UserIndex) { + newMap := make(map[string]int) + + idx.ForEachRecord(func(rec IndexUserRecord) bool { + if name := string(bytes.TrimRight(rec.CharacterName[:], "\x00")); name != `` { + newMap[strings.ToLower(name)] = int(rec.UserID) + } + return true + }) + + for _, u := range GetAllActiveUsers() { + if u.Character != nil && u.Character.Name != "" { + newMap[strings.ToLower(u.Character.Name)] = u.UserId + } + } + + ci.mu.Lock() + ci.byName = newMap + ci.mu.Unlock() +} + // RebuildFromScan is Rebuild fed by an existing user file scan, so startup // can share one scan between the user index and the character index instead // of fully parsing every user record a second time. diff --git a/internal/users/index.go b/internal/users/index.go index 87c94192b..04de05b19 100644 --- a/internal/users/index.go +++ b/internal/users/index.go @@ -8,6 +8,8 @@ import ( "hash/fnv" "io" "os" + "path/filepath" + "strconv" "strings" "sync" @@ -25,9 +27,10 @@ var ( ) const ( - IndexVersion = 2 + IndexVersion = 3 IndexLineTerminatorV1 = byte(10) // "\n" IndexRecordSizeV1 = 89 + IndexRecordSizeV3 = 185 // username[80] + userid(8) + charname[80] + mtime(8) + size(8) + newline FixedHeaderTotalLength = 100 // 99 bytes header content + 1 byte newline ) @@ -40,10 +43,16 @@ type IndexMetaData struct { Checksum uint64 // FNV-64 fingerprint of user directory contents; 0 when IndexChecksumEnabled is false } -// IndexUserRecord represents one fixed-width record. +// IndexUserRecord represents one fixed-width record. Alongside the lookup +// fields it stores the active character name plus the mtime and size the +// user file had when it was indexed, so startup can tell unchanged files +// apart from changed ones without opening them. type IndexUserRecord struct { - UserID int64 - Username [80]byte + UserID int64 + Username [80]byte + CharacterName [80]byte + FileModTime int64 // UnixNano mtime of the user file when last indexed + FileSize int64 // size in bytes of the user file when last indexed } // UserIndex is the central struct that holds the index filename and methods @@ -102,6 +111,14 @@ func (idx *UserIndex) loadRecords() { return } + // An index in an older record format is not readable - leave the maps + // empty so SyncWithUserFiles falls back to a full rebuild. + if idx.metaData.IndexVersion != IndexVersion || idx.metaData.RecordSize != IndexRecordSizeV3 { + mudlog.Info("UserIndex", "info", "index format is outdated, a rebuild will recreate it", "path", idx.Filename, "version", idx.metaData.IndexVersion) + idx.records = nil + return + } + f, err := os.Open(idx.Filename) if err != nil { mudlog.Error("UserIndex", "error", "failed to open index file", "path", idx.Filename, "details", err) @@ -127,6 +144,9 @@ func (idx *UserIndex) loadRecords() { rec := &idx.records[i] copy(rec.Username[:], buf[offset:offset+80]) rec.UserID = int64(binary.LittleEndian.Uint64(buf[offset+80 : offset+88])) + copy(rec.CharacterName[:], buf[offset+88:offset+168]) + rec.FileModTime = int64(binary.LittleEndian.Uint64(buf[offset+168 : offset+176])) + rec.FileSize = int64(binary.LittleEndian.Uint64(buf[offset+176 : offset+184])) username := string(bytes.TrimRight(rec.Username[:], "\x00")) idx.byUsername[username] = rec.UserID @@ -155,7 +175,7 @@ func (idx *UserIndex) Create() error { MetaDataSize: FixedHeaderTotalLength, IndexVersion: IndexVersion, RecordCount: 0, - RecordSize: IndexRecordSizeV1, + RecordSize: IndexRecordSizeV3, } idx.highestUserId = 0 idx.records = nil @@ -265,16 +285,31 @@ func (idx *UserIndex) RebuildFromScan(scan []UserFileScan) error { return idx.applyScan(scan, checksum) } +// scanRecord converts one scan result into a fixed-width index record. +func scanRecord(s UserFileScan) IndexUserRecord { + rec := IndexUserRecord{ + UserID: int64(s.UserId), + FileModTime: s.FileModTime, + FileSize: s.FileSize, + } + copy(rec.Username[:], strings.ToLower(s.Username)) + copy(rec.CharacterName[:], s.CharacterName) + return rec +} + // applyScan replaces the in-memory records and lookup maps with the scan // results, then writes the complete index to disk once. func (idx *UserIndex) applyScan(scan []UserFileScan, checksum uint64) error { - records := make([]IndexUserRecord, 0, len(scan)) for _, s := range scan { - rec := IndexUserRecord{UserID: int64(s.UserId)} - copy(rec.Username[:], strings.ToLower(s.Username)) - records = append(records, rec) + records = append(records, scanRecord(s)) } + return idx.applyRecords(records, checksum) +} + +// applyRecords replaces the in-memory records and lookup maps, then writes +// the complete index to disk once. +func (idx *UserIndex) applyRecords(records []IndexUserRecord, checksum uint64) error { idx.mu.Lock() defer idx.mu.Unlock() @@ -283,7 +318,7 @@ func (idx *UserIndex) applyScan(scan []UserFileScan, checksum uint64) error { MetaDataSize: FixedHeaderTotalLength, IndexVersion: IndexVersion, RecordCount: uint64(len(records)), - RecordSize: IndexRecordSizeV1, + RecordSize: IndexRecordSizeV3, Checksum: checksum, } @@ -409,10 +444,11 @@ func (idx *UserIndex) AddUser(userId int, username string) error { return fmt.Errorf("error seeking to file end: %w", err) } - var recBuf [IndexRecordSizeV1]byte - copy(recBuf[:80], newRecord.Username[:]) - binary.LittleEndian.PutUint64(recBuf[80:88], uint64(newRecord.UserID)) - recBuf[88] = IndexLineTerminatorV1 + // The character name, mtime, and size are left zeroed here - the user + // file usually doesn't exist yet when a user is first registered. The + // zero mtime never matches the real file, so the next startup sync + // re-parses that one file and completes the record. + recBuf := encodeIndexRecord(newRecord) if _, err := f.Write(recBuf[:]); err != nil { return fmt.Errorf("error writing record: %w", err) } @@ -496,6 +532,126 @@ func (m IndexMetaData) Format() ([]byte, error) { return []byte(padded + string(IndexLineTerminatorV1)), nil } +// SyncWithUserFiles brings the index in line with the user files on disk, +// parsing only files that are new or changed since they were last indexed +// and dropping records whose files are gone. Unchanged files are never +// opened. Returns how many records had to be parsed or dropped; when that +// is zero the index file is not rewritten at all. +func (idx *UserIndex) SyncWithUserFiles() (int, error) { + basePath := util.FilePath(string(configs.GetFilePathsConfig().DataFiles), `/`, `users`) + return idx.syncWithDir(basePath) +} + +// syncWithDir is SyncWithUserFiles parameterized by the directory to sync +// against, so tests can point it at a synthetic users directory. +func (idx *UserIndex) syncWithDir(basePath string) (int, error) { + + // An index that is missing, in an older format, or that did not load + // every record its header claims cannot be trusted for incremental + // work - fall back to a full scan of every user file. + idx.mu.RLock() + trustworthy := idx.metaData.IndexVersion == IndexVersion && + idx.metaData.RecordSize == IndexRecordSizeV3 && + uint64(len(idx.records)) == idx.metaData.RecordCount + oldRecords := idx.records + idx.mu.RUnlock() + + if !idx.Exists() { + trustworthy = false + } + + if !trustworthy { + scan := scanUserFilesInDir(basePath) + checksum, err := computeDirChecksum(basePath) + if err != nil { + return 0, fmt.Errorf("checksum compute failed: %w", err) + } + return len(scan), idx.applyScan(scan, checksum) + } + + entries, err := os.ReadDir(basePath) + if err != nil { + return 0, fmt.Errorf("users directory unreadable: %w", err) + } + + onDisk := make(map[string]os.FileInfo, len(entries)) + for _, e := range entries { + if e.IsDir() { + continue + } + name := e.Name() + if !strings.HasSuffix(name, `.yaml`) || strings.HasSuffix(name, `.alts.yaml`) { + continue + } + info, err := e.Info() + if err != nil { + return 0, fmt.Errorf("stat failed for %s: %w", name, err) + } + onDisk[name] = info + } + + // Records are matched to files by the .yaml naming convention. + // A user file in the old username.yaml naming never matches a record, + // so it is re-parsed every startup - run the format migration to avoid + // that cost. + changed := 0 + kept := make([]IndexUserRecord, 0, len(oldRecords)) + reparse := []string{} + claimed := make(map[string]bool, len(oldRecords)) + + for _, rec := range oldRecords { + fileName := strconv.FormatInt(rec.UserID, 10) + `.yaml` + info, ok := onDisk[fileName] + if !ok { + changed++ + continue + } + claimed[fileName] = true + if info.ModTime().UnixNano() == rec.FileModTime && info.Size() == rec.FileSize { + kept = append(kept, rec) + continue + } + reparse = append(reparse, fileName) + changed++ + } + + for name := range onDisk { + if !claimed[name] { + reparse = append(reparse, name) + changed++ + } + } + + if changed == 0 { + return 0, nil + } + + for _, name := range reparse { + if s, ok := scanUserFile(filepath.Join(basePath, name), onDisk[name]); ok { + kept = append(kept, scanRecord(s)) + } + } + + checksum, err := computeDirChecksum(basePath) + if err != nil { + return 0, fmt.Errorf("checksum compute failed: %w", err) + } + + return changed, idx.applyRecords(kept, checksum) +} + +// encodeIndexRecord serializes one record into its fixed-width on-disk form. +func encodeIndexRecord(rec IndexUserRecord) [IndexRecordSizeV3]byte { + var recBuf [IndexRecordSizeV3]byte + copy(recBuf[:80], rec.Username[:]) + binary.LittleEndian.PutUint64(recBuf[80:88], uint64(rec.UserID)) + copy(recBuf[88:168], rec.CharacterName[:]) + binary.LittleEndian.PutUint64(recBuf[168:176], uint64(rec.FileModTime)) + binary.LittleEndian.PutUint64(recBuf[176:184], uint64(rec.FileSize)) + recBuf[184] = IndexLineTerminatorV1 + return recBuf +} + // writeCompleteIndex writes metadata and all records atomically via temp file + rename. func (idx *UserIndex) writeCompleteIndex(records []IndexUserRecord) error { tmpFile := idx.Filename + ".tmp" @@ -510,14 +666,11 @@ func (idx *UserIndex) writeCompleteIndex(records []IndexUserRecord) error { return err } - buf := make([]byte, 0, len(headerBytes)+len(records)*IndexRecordSizeV1) + buf := make([]byte, 0, len(headerBytes)+len(records)*IndexRecordSizeV3) buf = append(buf, headerBytes...) - var recBuf [IndexRecordSizeV1]byte for _, rec := range records { - copy(recBuf[:80], rec.Username[:]) - binary.LittleEndian.PutUint64(recBuf[80:88], uint64(rec.UserID)) - recBuf[88] = IndexLineTerminatorV1 + recBuf := encodeIndexRecord(rec) buf = append(buf, recBuf[:]...) } diff --git a/internal/users/indexscan.go b/internal/users/indexscan.go index 76aa77e16..84778de24 100644 --- a/internal/users/indexscan.go +++ b/internal/users/indexscan.go @@ -14,11 +14,15 @@ import ( // UserFileScan holds the fields a lightweight pass over a user file yields. // The user index and the character index are both built from these at -// startup, without paying for a full UserRecord unmarshal per file. +// startup, without paying for a full UserRecord unmarshal per file. The +// mtime and size record what the file looked like when it was read, so a +// later sync can tell unchanged files apart without opening them. type UserFileScan struct { UserId int Username string CharacterName string + FileModTime int64 + FileSize int64 } // userFileScanFields is the minimal unmarshal target for a scan. The file @@ -66,45 +70,61 @@ func scanUserFilesInDir(basePath string) []UserFileScan { return nil } - fileBytes, err := os.ReadFile(path) - if err != nil { - mudlog.Warn("ScanUserFiles", "path", path, "read_error", err) + scan, ok := scanUserFile(path, info) + if !ok { return nil } - var scanned userFileScanFields - if err := yaml.Unmarshal(fileBytes, &scanned); err != nil { - mudlog.Warn("ScanUserFiles", "path", path, "unmarshal_error", err) - return nil - } - - if scanned.UserId < 1 || scanned.Username == `` { - mudlog.Warn("ScanUserFiles", "info", "skipping user file missing userid or username", "path", path) - return nil + if fileId, convErr := strconv.Atoi(strings.TrimSuffix(filepath.Base(path), `.yaml`)); convErr == nil && fileId != scan.UserId { + mudlog.Warn("ScanUserFiles", "info", "filename does not match userid in file", "path", path, "userid", scan.UserId) } - if fileId, convErr := strconv.Atoi(strings.TrimSuffix(filepath.Base(path), `.yaml`)); convErr == nil && fileId != scanned.UserId { - mudlog.Warn("ScanUserFiles", "info", "filename does not match userid in file", "path", path, "userid", scanned.UserId) + if otherPath, ok := seenIds[scan.UserId]; ok { + mudlog.Warn("ScanUserFiles", "info", "duplicate userid", "userid", scan.UserId, "path", path, "otherpath", otherPath) } - - if otherPath, ok := seenIds[scanned.UserId]; ok { - mudlog.Warn("ScanUserFiles", "info", "duplicate userid", "userid", scanned.UserId, "path", path, "otherpath", otherPath) - } - lowerName := strings.ToLower(scanned.Username) + lowerName := strings.ToLower(scan.Username) if otherPath, ok := seenNames[lowerName]; ok { - mudlog.Warn("ScanUserFiles", "info", "duplicate username", "username", scanned.Username, "path", path, "otherpath", otherPath) + mudlog.Warn("ScanUserFiles", "info", "duplicate username", "username", scan.Username, "path", path, "otherpath", otherPath) } - seenIds[scanned.UserId] = path + seenIds[scan.UserId] = path seenNames[lowerName] = path - results = append(results, UserFileScan{ - UserId: scanned.UserId, - Username: scanned.Username, - CharacterName: scanned.Character.Name, - }) + results = append(results, scan) return nil }) return results } + +// scanUserFile reads and minimally parses one user file. The provided info +// supplies the mtime and size stored with the result - stat data from +// before the read, so a write racing the scan makes the file look changed +// on the next sync rather than silently current. +func scanUserFile(path string, info os.FileInfo) (UserFileScan, bool) { + + fileBytes, err := os.ReadFile(path) + if err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "read_error", err) + return UserFileScan{}, false + } + + var scanned userFileScanFields + if err := yaml.Unmarshal(fileBytes, &scanned); err != nil { + mudlog.Warn("ScanUserFiles", "path", path, "unmarshal_error", err) + return UserFileScan{}, false + } + + if scanned.UserId < 1 || scanned.Username == `` { + mudlog.Warn("ScanUserFiles", "info", "skipping user file missing userid or username", "path", path) + return UserFileScan{}, false + } + + return UserFileScan{ + UserId: scanned.UserId, + Username: scanned.Username, + CharacterName: scanned.Character.Name, + FileModTime: info.ModTime().UnixNano(), + FileSize: info.Size(), + }, true +} diff --git a/internal/users/indexsync_test.go b/internal/users/indexsync_test.go new file mode 100644 index 000000000..f7e3bff03 --- /dev/null +++ b/internal/users/indexsync_test.go @@ -0,0 +1,287 @@ +package users + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoMudEngine/GoMud/internal/mudlog" +) + +// syncTestDir writes count realistic user files and returns a synced index +// for them. +func syncTestDir(t testing.TB, count int) (string, *UserIndex) { + t.Helper() + + dir := t.TempDir() + for i := 1; i <= count; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + idx := newScanTestIndex(dir) + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("initial sync failed: %v", err) + } + if changed != count { + t.Fatalf("expected initial sync to parse %d files, got %d", count, changed) + } + return dir, idx +} + +// TestSyncNoChangesSkipsWrite verifies a second sync over an unchanged +// directory parses nothing and leaves the index file untouched. +func TestSyncNoChangesSkipsWrite(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 10) + + before, err := os.Stat(idx.Filename) + if err != nil { + t.Fatal(err) + } + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("second sync failed: %v", err) + } + if changed != 0 { + t.Errorf("expected 0 changed on unchanged directory, got %d", changed) + } + + after, err := os.Stat(idx.Filename) + if err != nil { + t.Fatal(err) + } + if !after.ModTime().Equal(before.ModTime()) || after.Size() != before.Size() { + t.Error("index file was rewritten despite no changes") + } +} + +// TestSyncDetectsChangedFile verifies a modified user file is re-parsed and +// its index entry updated, without disturbing the others. +func TestSyncDetectsChangedFile(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 10) + + // Rename the user inside file 3. The content length changes, so the + // size comparison alone must catch it even with coarse mtimes. + writeScanTestUser(t, dir, 3, `Renamed_User_Three`, `Hero_3_Reborn`, 2) + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 1 { + t.Errorf("expected 1 changed record, got %d", changed) + } + + if userId, found := idx.FindByUsername(`renamed_user_three`); !found || userId != 3 { + t.Errorf("expected renamed user findable with userId 3, got %d, found=%v", userId, found) + } + if _, found := idx.FindByUsername(`user_3`); found { + t.Error("old username should be gone after re-parse") + } + if userId, found := idx.FindByUsername(`user_7`); !found || userId != 7 { + t.Errorf("untouched user_7 should remain, got %d, found=%v", userId, found) + } +} + +// TestSyncDetectsNewAndDeleted verifies new files are indexed and records +// for deleted files are dropped. +func TestSyncDetectsNewAndDeleted(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 10) + + writeScanTestUser(t, dir, 11, `User_11`, `Hero_11`, 1) + if err := os.Remove(filepath.Join(dir, `2.yaml`)); err != nil { + t.Fatal(err) + } + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 2 { + t.Errorf("expected 2 changed records (one new, one dropped), got %d", changed) + } + + if userId, found := idx.FindByUsername(`user_11`); !found || userId != 11 { + t.Errorf("expected new user_11 indexed, got %d, found=%v", userId, found) + } + if _, found := idx.FindByUserId(2); found { + t.Error("deleted user 2 should be gone from the index") + } + if highest := idx.GetHighestUserId(); highest != 11 { + t.Errorf("expected highest userId 11, got %d", highest) + } +} + +// TestSyncUpgradesOldFormatIndex verifies an index in the previous record +// format triggers a full rebuild into the current format. +func TestSyncUpgradesOldFormatIndex(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir := t.TempDir() + for i := 1; i <= 5; i++ { + writeScanTestUser(t, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 1) + } + + // Hand-craft a version 2 index: 89-byte records, no character name. + header := `VERSION=2,RECORDCOUNT=1,RECORDSIZE=89,CHECKSUM=12345` + header = header + strings.Repeat(` `, FixedHeaderTotalLength-1-len(header)) + "\n" + record := make([]byte, 89) + copy(record[:80], `staleuser`) + record[80] = 42 // userid 42, little-endian low byte + record[88] = IndexLineTerminatorV1 + idxPath := filepath.Join(dir, `users.idx`) + if err := os.WriteFile(idxPath, append([]byte(header), record...), 0644); err != nil { + t.Fatal(err) + } + + idx := newScanTestIndex(dir) + idx.metaData = idx.getMetaDataFromFile() + idx.loadRecords() + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 5 { + t.Errorf("expected full rebuild of 5 users after format upgrade, got %d", changed) + } + + if _, found := idx.FindByUsername(`staleuser`); found { + t.Error("record from the old-format index should not survive the upgrade") + } + if meta := idx.GetMetaData(); meta.IndexVersion != IndexVersion || meta.RecordSize != IndexRecordSizeV3 { + t.Errorf("expected index upgraded to version %d with record size %d, got %+v", IndexVersion, IndexRecordSizeV3, meta) + } +} + +// TestRebuildFromIndexReadsNoFiles verifies the character index can be +// rebuilt purely from index records: after deleting every user file, the +// names must still resolve. +func TestRebuildFromIndexReadsNoFiles(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 5) + + files, err := filepath.Glob(filepath.Join(dir, `*.yaml`)) + if err != nil { + t.Fatal(err) + } + for _, f := range files { + if err := os.Remove(f); err != nil { + t.Fatal(err) + } + } + + ci := freshCharacterIndex() + orig := characterIndex + characterIndex = ci + defer func() { characterIndex = orig }() + + ci.RebuildFromIndex(idx) + + if ci.Len() != 5 { + t.Fatalf("expected 5 characters from index records, got %d", ci.Len()) + } + if userId, found := ci.Find(`Hero_4`); !found || userId != 4 { + t.Errorf("expected Hero_4 to resolve to userId 4, got %d, found=%v", userId, found) + } +} + +// TestSyncCompletesAddUserStub verifies the runtime AddUser record (zero +// mtime and size) is re-parsed and completed by the next sync. +func TestSyncCompletesAddUserStub(t *testing.T) { + mudlog.SetupLogger(nil, "", "", false) + + dir, idx := syncTestDir(t, 3) + + // Simulate a runtime registration: index entry first, file second. + if err := idx.AddUser(4, `Newcomer`); err != nil { + t.Fatal(err) + } + writeScanTestUser(t, dir, 4, `Newcomer`, `Fresh_Hero`, 1) + + changed, err := idx.syncWithDir(dir) + if err != nil { + t.Fatalf("sync failed: %v", err) + } + if changed != 1 { + t.Errorf("expected exactly the stub record to be re-parsed, got %d changed", changed) + } + + ci := freshCharacterIndex() + ci.RebuildFromIndex(idx) + if userId, found := ci.Find(`Fresh_Hero`); !found || userId != 4 { + t.Errorf("expected completed record to carry character name, got %d, found=%v", userId, found) + } +} + +// BenchmarkSyncNoChanges measures the steady-state startup cost: a sync +// over a directory where nothing changed since the last index write. +func BenchmarkSyncNoChanges(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + for _, userCt := range []int{100, 1000, 10000} { + b.Run(fmt.Sprintf(`%d_users`, userCt), func(b *testing.B) { + dir := b.TempDir() + for i := 1; i <= userCt; i++ { + writeScanTestUser(b, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 8) + } + idx := newScanTestIndex(dir) + if _, err := idx.syncWithDir(dir); err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + changed, err := idx.syncWithDir(dir) + if err != nil { + b.Fatal(err) + } + if changed != 0 { + b.Fatalf("expected steady state, got %d changed", changed) + } + } + }) + } +} + +// BenchmarkSyncWithChurn measures a sync where 50 of 1000 user files +// changed since the last index write - the realistic restart shape. +func BenchmarkSyncWithChurn(b *testing.B) { + mudlog.SetupLogger(nil, "", "", false) + + dir := b.TempDir() + for i := 1; i <= 1000; i++ { + writeScanTestUser(b, dir, i, fmt.Sprintf(`User_%d`, i), fmt.Sprintf(`Hero_%d`, i), 8) + } + idx := newScanTestIndex(dir) + if _, err := idx.syncWithDir(dir); err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + for u := 1; u <= 50; u++ { + writeScanTestUser(b, dir, u, fmt.Sprintf(`User_%d`, u), fmt.Sprintf(`Hero_%d_v%d`, u, i), 8) + } + b.StartTimer() + + changed, err := idx.syncWithDir(dir) + if err != nil { + b.Fatal(err) + } + if changed != 50 { + b.Fatalf("expected 50 changed, got %d", changed) + } + } +} diff --git a/main.go b/main.go index f5dd115c0..fb7969fd3 100644 --- a/main.go +++ b/main.go @@ -261,34 +261,25 @@ func main() { // Create the user index isCopyover := flags.CopyoverFd() >= 0 - if !isCopyover { - idx := users.InitUserIndex() - if !idx.Exists() { - // Since it doesn't exist yet, that's a good indication we should do a quick format migration check - users.DoUserMigrations() - } + syncStart := time.Now() + idx := users.InitUserIndex() + if !isCopyover && !idx.Exists() { + // Since it doesn't exist yet, that's a good indication we should do a quick format migration check + users.DoUserMigrations() } - // One lightweight scan of the user files (userid, username, character - // name) feeds both the user index and the character index, instead of - // each index fully parsing every user record on its own. - scanStart := time.Now() - userScan := users.ScanUserFiles() - - if !isCopyover { - idx := users.GetUserIndex() - if idx.IsUpToDate() { - mudlog.Info("UserIndex", "info", "User index up to date.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(scanStart)) - } else { - if err := idx.RebuildFromScan(userScan); err != nil { - mudlog.Error("UserIndex", "error", "rebuild failed", "details", err) - } - mudlog.Info("UserIndex", "info", "User index recreated.", "users", idx.GetMetaData().RecordCount, "time taken", time.Since(scanStart)) - } + // Bring the user index in line with the user files, parsing only files + // that changed since they were last indexed. The character index is then + // rebuilt straight from the index records, so an unchanged user file is + // never opened at all. + changed, err := idx.SyncWithUserFiles() + if err != nil { + mudlog.Error("UserIndex", "error", "sync failed", "details", err) } + mudlog.Info("UserIndex", "info", "User index synced.", "users", idx.GetMetaData().RecordCount, "changed", changed, "time taken", time.Since(syncStart)) - users.GetCharacterIndex().RebuildFromScan(userScan) - mudlog.Info("CharacterIndex", "info", "Active character names indexed.", "characters", users.GetCharacterIndex().Len(), "time taken", time.Since(scanStart)) + users.GetCharacterIndex().RebuildFromIndex(idx) + mudlog.Info("CharacterIndex", "info", "Active character names indexed.", "characters", users.GetCharacterIndex().Len(), "time taken", time.Since(syncStart)) // Load the round count from the file if !isCopyover {