Skip to content

Commit ababea9

Browse files
furiosaBen Baumgold
authored andcommitted
fix(EnsureAllMetadata): prevent ping-pong when two databases map to same rig (gt-cn4)
When a rig has two Dolt databases (e.g., 'gastown' and 'gt' both mapping to the gastown rig via routes.jsonl), EnsureAllMetadata previously called EnsureMetadata for each database independently. Each call overwrote the other's dolt_database value, producing alternating correcting warnings in 'gt up' output. Fix: group databases by rig before processing. For rigs with multiple databases, read the rig's existing metadata.json — if it already points to one of the valid databases, keep it (stable, no rewrite). Only when no stable value exists fall back to the lexicographically first candidate for determinism. Each rig is now processed exactly once. Add TestEnsureAllMetadata_NoPingPong regression test covering the two-DB scenario for the same rig.
1 parent a461142 commit ababea9

2 files changed

Lines changed: 129 additions & 0 deletions

File tree

internal/doltserver/doltserver.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"path/filepath"
4040

4141
"runtime"
42+
"sort"
4243
"strconv"
4344
"strings"
4445
"sync"
@@ -2977,6 +2978,12 @@ func EnsureAllMetadata(townRoot string) (updated []string, errs []error) {
29772978
dbToRig[k] = v
29782979
}
29792980

2981+
// Group databases by rig name to detect collisions (e.g., "gastown" and
2982+
// "gt" both mapping to the "gastown" rig). Preserve encounter order so
2983+
// the result slice is deterministic.
2984+
rigToDbs := make(map[string][]string)
2985+
var rigOrder []string
2986+
seenRig := make(map[string]bool)
29802987
for _, dbName := range databases {
29812988
rigName := dbName
29822989
if mapped, ok := dbToRig[dbName]; ok {
@@ -2986,6 +2993,19 @@ func EnsureAllMetadata(townRoot string) (updated []string, errs []error) {
29862993
if dbName == "hq" {
29872994
rigName = "hq"
29882995
}
2996+
rigToDbs[rigName] = append(rigToDbs[rigName], dbName)
2997+
if !seenRig[rigName] {
2998+
seenRig[rigName] = true
2999+
rigOrder = append(rigOrder, rigName)
3000+
}
3001+
}
3002+
3003+
// For each rig, choose exactly one canonical database. When multiple
3004+
// databases map to the same rig (e.g., "gastown" and "gt" for the gastown
3005+
// rig), prefer whichever database metadata.json already records — this
3006+
// prevents ping-pong where each database's turn overwrites the other's value.
3007+
for _, rigName := range rigOrder {
3008+
dbName := chooseCanonicalDB(townRoot, rigName, rigToDbs[rigName])
29893009
// Pass dbName explicitly so EnsureMetadata writes the correct
29903010
// dolt_database value ("be") rather than the rig dir name ("beads_el").
29913011
if err := EnsureMetadata(townRoot, rigName, dbName); err != nil {
@@ -2998,6 +3018,38 @@ func EnsureAllMetadata(townRoot string) (updated []string, errs []error) {
29983018
return updated, errs
29993019
}
30003020

3021+
// chooseCanonicalDB picks the single database name to use for a rig when
3022+
// multiple Dolt databases map to the same rig directory. It reads the rig's
3023+
// existing metadata.json and returns the current dolt_database value if it is
3024+
// in the candidate list (stable — avoids rewriting metadata.json). If no
3025+
// stable choice exists it returns the lexicographically first candidate.
3026+
func chooseCanonicalDB(townRoot, rigName string, candidates []string) string {
3027+
if len(candidates) == 1 {
3028+
return candidates[0]
3029+
}
3030+
// Build a set for O(1) lookup.
3031+
valid := make(map[string]bool, len(candidates))
3032+
for _, db := range candidates {
3033+
valid[db] = true
3034+
}
3035+
// Check what metadata.json currently records.
3036+
beadsDir := FindRigBeadsDir(townRoot, rigName)
3037+
metadataPath := filepath.Join(beadsDir, "metadata.json")
3038+
if data, readErr := os.ReadFile(metadataPath); readErr == nil {
3039+
var meta map[string]interface{}
3040+
if json.Unmarshal(data, &meta) == nil {
3041+
if current, ok := meta["dolt_database"].(string); ok && valid[current] {
3042+
return current // already stable — keep it
3043+
}
3044+
}
3045+
}
3046+
// No stable value — pick lexicographically first for determinism.
3047+
sorted := make([]string, len(candidates))
3048+
copy(sorted, candidates)
3049+
sort.Strings(sorted)
3050+
return sorted[0]
3051+
}
3052+
30013053
// buildDatabaseToRigMap loads routes.jsonl and builds a map from database name
30023054
// (prefix without hyphen) to rig name (first component of the path).
30033055
// For example: "bd" -> "beads", "gt" -> "gastown", "sw" -> "sallaWork"

internal/doltserver/doltserver_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4164,6 +4164,83 @@ func TestEnsureAllMetadata_FallbackToDbName(t *testing.T) {
41644164
}
41654165
}
41664166

4167+
// TestEnsureAllMetadata_NoPingPong is a regression test for the bug where two
4168+
// Dolt databases map to the same rig (e.g., "gastown" and "gt" both resolving
4169+
// to the "gastown" rig). Previously, each call to EnsureMetadata overwrote the
4170+
// other's dolt_database value on every invocation of EnsureAllMetadata, causing
4171+
// alternating "correcting" warnings in gt up output.
4172+
//
4173+
// Expected: EnsureAllMetadata calls EnsureMetadata exactly once per rig,
4174+
// and the chosen database is stable across repeated calls.
4175+
func TestEnsureAllMetadata_NoPingPong(t *testing.T) {
4176+
townRoot := t.TempDir()
4177+
dataDir := filepath.Join(townRoot, ".dolt-data")
4178+
4179+
// Both "gastown" and "gt" databases exist.
4180+
setupDoltDB(t, dataDir, "gastown")
4181+
setupDoltDB(t, dataDir, "gt")
4182+
4183+
// routes.jsonl maps prefix "gt-" → rig "gastown".
4184+
beadsDir := filepath.Join(townRoot, ".beads")
4185+
if err := os.MkdirAll(beadsDir, 0755); err != nil {
4186+
t.Fatal(err)
4187+
}
4188+
routesContent := `{"prefix":"gt-","path":"gastown/mayor/rig"}
4189+
`
4190+
if err := os.WriteFile(filepath.Join(beadsDir, "routes.jsonl"), []byte(routesContent), 0644); err != nil {
4191+
t.Fatal(err)
4192+
}
4193+
4194+
// Create gastown rig beads directory.
4195+
gastownBeads := filepath.Join(townRoot, "gastown", "mayor", "rig", ".beads")
4196+
if err := os.MkdirAll(gastownBeads, 0755); err != nil {
4197+
t.Fatal(err)
4198+
}
4199+
4200+
// First call: no prior metadata — a canonical database is chosen.
4201+
updated1, errs1 := EnsureAllMetadata(townRoot)
4202+
if len(errs1) > 0 {
4203+
t.Fatalf("first call: unexpected errors: %v", errs1)
4204+
}
4205+
// Both databases exist but map to the same rig → only one update expected.
4206+
if len(updated1) != 1 {
4207+
t.Errorf("first call: expected 1 updated entry (one per rig), got %d: %v", len(updated1), updated1)
4208+
}
4209+
4210+
// Read what was written.
4211+
meta1 := readMetadataJSON(t, filepath.Join(gastownBeads, "metadata.json"))
4212+
chosen := meta1["dolt_database"].(string)
4213+
if chosen != "gastown" && chosen != "gt" {
4214+
t.Fatalf("first call: unexpected dolt_database %q", chosen)
4215+
}
4216+
4217+
// Second call: metadata already set — same database should be kept.
4218+
updated2, errs2 := EnsureAllMetadata(townRoot)
4219+
if len(errs2) > 0 {
4220+
t.Fatalf("second call: unexpected errors: %v", errs2)
4221+
}
4222+
meta2 := readMetadataJSON(t, filepath.Join(gastownBeads, "metadata.json"))
4223+
if meta2["dolt_database"] != chosen {
4224+
t.Errorf("ping-pong detected: first call chose %q, second call chose %q",
4225+
chosen, meta2["dolt_database"])
4226+
}
4227+
_ = updated2 // may be 0 or 1 depending on whether anything changed
4228+
}
4229+
4230+
// readMetadataJSON reads and parses a metadata.json file, failing the test on error.
4231+
func readMetadataJSON(t *testing.T, path string) map[string]interface{} {
4232+
t.Helper()
4233+
data, err := os.ReadFile(path)
4234+
if err != nil {
4235+
t.Fatalf("reading metadata.json at %s: %v", path, err)
4236+
}
4237+
var meta map[string]interface{}
4238+
if err := json.Unmarshal(data, &meta); err != nil {
4239+
t.Fatalf("parsing metadata.json at %s: %v", path, err)
4240+
}
4241+
return meta
4242+
}
4243+
41674244
func TestCleanStaleSocket_RemovesStaleFile(t *testing.T) {
41684245
if runtime.GOOS == "windows" {
41694246
t.Skip("Unix sockets not applicable on Windows")

0 commit comments

Comments
 (0)