Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions rest/upgradetest/upgrade_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ func TestLegacyMetadataID(t *testing.T) {
resp := legacyRT.SendAdminRequest("PUT", "/db/testLegacyMetadataID", `{"test":"test"}`)
rest.RequireStatus(t, resp, http.StatusCreated)

// Uncomment to repro CBG-5038
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment explaining what timing issue this reproduces and why waiting for attachment migration completion is necessary. This will help future developers understand the context when they uncomment this code as part of CBG-5038.

Suggested change
// Uncomment to repro CBG-5038
// CBG-5038: The legacy database may start the attachment migration background process after this write.
// Waiting here for attachment migration to complete forces a specific timing: the migration finishes
// before we read out the legacy config and recreate the database using persistent config. That ordering
// exposed a bug in how legacy metadata IDs and attachment migration state were handled across upgrade.
// This call is intentionally left commented out during normal test runs, but can be uncommented when
// explicitly reproducing or debugging CBG-5038 to ensure the test waits for migration completion.

Copilot uses AI. Check for mistakes.
// legacyRT.WaitForAttachmentMigrationStatus(t, db.BackgroundProcessStateCompleted)

dbConfigString := getDbConfigFromLegacyConfig(legacyRT)
legacyRT.Close()

Expand Down Expand Up @@ -266,6 +269,8 @@ func TestMetadataIDWithConfigGroups(t *testing.T) {
resp := legacyRT.SendAdminRequest("PUT", "/db/testLegacyMetadataID", `{"test":"test"}`)
assert.Equal(t, http.StatusCreated, resp.Code)

// Uncomment to repro CBG-5038
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment explaining what timing issue this reproduces and why waiting for attachment migration completion is necessary. This will help future developers understand the context when they uncomment this code as part of CBG-5038.

Suggested change
// Uncomment to repro CBG-5038
// This wait controls the relative timing between legacy attachment migration and the creation of
// the grouped databases below. CBG-5038 tracks a race where the metadata ID handling differs
// depending on whether attachment migration has already completed by the time the new database
// instances (in different config groups) are brought online. Uncommenting this line forces the
// test to wait for attachment migration to reach BackgroundProcessStateCompleted before creating
// the group-based databases, in order to reproduce/verify the CBG-5038 timing scenario.

Copilot uses AI. Check for mistakes.
// legacyRT.WaitForAttachmentMigrationStatus(t, db.BackgroundProcessStateCompleted)
dbConfigString := getDbConfigFromLegacyConfig(legacyRT)
legacyRT.Close()

Expand Down
Loading