Skip to content

Update the type of server when its service gets deleted #2902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
h2zh merged 1 commit into from
Jan 14, 2026
Merged

Update the type of server when its service gets deleted #2902

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 40 additions & 8 deletions registry/registry_db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,7 @@ func setRegistrationPubKey(prefix string, pubkeyDbString string) error {
// remove the corresponding entry in the “services” table.
// Additionally, if this server only has this single service registration (i.e., the server is only an
// origin or only a cache, not both), then delete the corresponding entry in the “servers” table.
// If services remain, update the server's is_origin and is_cache fields accordingly.
func deleteRegistrationByID(id int) error {
// Wrap in a transaction to perform an atomic operation
return database.ServerDatabase.Transaction(func(tx *gorm.DB) error {
Expand All @@ -805,17 +806,48 @@ func deleteRegistrationByID(id int) error {
return err
}

// Conditionally delete the server atomically only if no services remain
if svc.ServerID != "" {
// The nested NOT EXISTS subquery ensures we only delete the server if
// there are no remaining rows in `services` referencing this server. Within
// the same transaction, this makes the check-and-delete atomic: if a new
// service is inserted concurrently, the subquery returns a row and the
// DELETE is skipped.
subq := tx.Model(&server_structs.Service{}).Select("1").Where("server_id = ?", svc.ServerID).Limit(1)
if err := tx.Where("id = ? AND NOT EXISTS (?)", svc.ServerID, subq).Delete(&server_structs.Server{}).Error; err != nil {
var remainingSvcs []server_structs.Service
if err := tx.Where("server_id = ?", svc.ServerID).Preload("Registration").Find(&remainingSvcs).Error; err != nil {
return err
}

if len(remainingSvcs) == 0 {
// Also delete the server only if no services belonging to it remain.
//
// The nested NOT EXISTS subquery ensures we only delete the server if
// there are no remaining rows in `services` referencing this server. Within
// the same transaction, this makes the check-and-delete atomic: if a new
// service is inserted concurrently, the subquery returns a row and the
// DELETE is skipped.
subq := tx.Model(&server_structs.Service{}).Select("1").Where("server_id = ?", svc.ServerID).Limit(1)
if err := tx.Where("id = ? AND NOT EXISTS (?)", svc.ServerID, subq).Delete(&server_structs.Server{}).Error; err != nil {
return err
}
} else {
// Update server flags to reflect remaining service types
var hasOrigin, hasCache bool
for _, remaining := range remainingSvcs {
if remaining.Registration.ID == 0 {
Copy link
Contributor

@CannonLock CannonLock Jan 14, 2026

Choose a reason for hiding this comment

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

Why are you skipping if the ID is 0?

Copy link
Contributor Author

@h2zh h2zh Jan 14, 2026

Choose a reason for hiding this comment

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

Good question! This is a defensive guard against data inconsistencies. When line 811 calls tx.Where("server_id = ?", svc.ServerID).Preload("Registration").Find(&remainingSvcs), GORM attempts to load the associated Registration record for each Service. If for some reason the foreign constraint fails (unlikely to happen), a Service row references a registration_id that no longer exists in the registrations table, GORM will return a zero-value Registration struct, whose ID is 0.

You can also find a similar guard in buildServerRegistration function.

	for _, service := range services {
		// Exclude services whose preloaded Registration is missing
		if service.Registration.ID != 0 && service.Registration.Prefix != "" {
			result.Registration = append(result.Registration, service.Registration)
		}
	}

Copy link
Contributor

@CannonLock CannonLock Jan 14, 2026

Choose a reason for hiding this comment

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

@h2zh Is it at all possible for the foreign key constraint to fail with the current settings?

Copy link
Contributor Author

@h2zh h2zh Jan 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'd say none! The problem was fixed in https://opensciencegrid.atlassian.net/browse/OPS-439, #2748

continue
}
prefix := remaining.Registration.Prefix
if strings.HasPrefix(prefix, server_structs.OriginPrefix.String()) {
hasOrigin = true
}
if strings.HasPrefix(prefix, server_structs.CachePrefix.String()) {
hasCache = true
}
}
updates := map[string]interface{}{
"is_origin": hasOrigin,
"is_cache": hasCache,
"updated_at": time.Now(),
}
if err := tx.Model(&server_structs.Server{}).Where("id = ?", svc.ServerID).Updates(updates).Error; err != nil {
return err
}
}
}

return nil
Expand Down
Loading