diff --git a/pkg/scd/operational_intents_handler.go b/pkg/scd/operational_intents_handler.go index a09fcb74c..f1972b966 100644 --- a/pkg/scd/operational_intents_handler.go +++ b/pkg/scd/operational_intents_handler.go @@ -92,11 +92,6 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest "OperationalIntent owned by %s, but %s attempted to delete", old.Manager, *req.Auth.ClientID) } - removeImplicitSubscription, err := subscriptionCanBeRemoved(ctx, r, old.SubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Could not determine if Subscription can be removed") - } - // Find Subscriptions that may overlap the OperationalIntent's Volume4D allsubs, err := r.SearchSubscriptions(ctx, &dssmodels.Volume4D{ StartTime: old.StartTime, @@ -130,15 +125,6 @@ func (a *Server) DeleteOperationalIntentReference(ctx context.Context, req *rest return stacktrace.Propagate(err, "Unable to delete OperationalIntent from repo") } - // removeImplicitSubscription is only true if the OIR had a subscription defined - if removeImplicitSubscription { - // Automatically remove a now-unused implicit Subscription - err = r.DeleteSubscription(ctx, *old.SubscriptionID) - if err != nil { - return stacktrace.Propagate(err, "Unable to delete associated implicit Subscription") - } - } - // Return response to client response = &restapi.ChangeOperationalIntentReferenceResponse{ OperationalIntentReference: *old.ToRest(), diff --git a/pkg/scd/store/cockroach/operational_intents.go b/pkg/scd/store/cockroach/operational_intents.go index e98d3d280..22bceb9c2 100644 --- a/pkg/scd/store/cockroach/operational_intents.go +++ b/pkg/scd/store/cockroach/operational_intents.go @@ -146,10 +146,32 @@ func (s *repo) GetOperationalIntent(ctx context.Context, id dssmodels.ID) (*scdm func (s *repo) DeleteOperationalIntent(ctx context.Context, id dssmodels.ID) error { var ( deleteOperationQuery = ` - DELETE FROM + WITH deleted_oir AS ( + DELETE FROM scd_operations WHERE id = $1 + RETURNING + id, subscription_id + ), + exists_and_is_implicit AS ( + SELECT subscription_id + FROM deleted_oir + JOIN scd_subscriptions ON scd_subscriptions.id = deleted_oir.subscription_id + WHERE scd_subscriptions.implicit = true + ), + dependent_oirs AS ( -- NOTE: this sub-query will still return the OIR being deleted (!) + SELECT id + FROM scd_operations + WHERE subscription_id = (SELECT subscription_id FROM deleted_oir) + ), + deleted_subscription_id AS ( + DELETE FROM scd_subscriptions + WHERE id = (SELECT subscription_id FROM exists_and_is_implicit) + AND (SELECT COUNT(*) FROM dependent_oirs) = 1 -- NOTE: see above, the OIR being removed is still counted here, hence a value of 1 + RETURNING id + ) + SELECT id FROM deleted_oir ` ) @@ -157,13 +179,28 @@ func (s *repo) DeleteOperationalIntent(ctx context.Context, id dssmodels.ID) err if err != nil { return stacktrace.Propagate(err, "Failed to convert id to PgUUID") } - res, err := s.q.Exec(ctx, deleteOperationQuery, uid) + res, err := s.q.Query(ctx, deleteOperationQuery, uid) if err != nil { return stacktrace.Propagate(err, "Error in query: %s", deleteOperationQuery) } + defer res.Close() - if res.RowsAffected() == 0 { - return stacktrace.NewError("Could not delete Operation that does not exist") + // Check that the deleted OIR ID was returned: + var opID dssmodels.ID + if res.Next() { + err = res.Scan(&opID) + if err != nil { + return stacktrace.Propagate(err, "Error scanning deleted Operation ID") + } + } else if resErr := res.Err(); resErr != nil { + // Note: typically, res.Next() will be false and res.Err() non-nil when the query fails + // because it collided with another query and the whole transaction needs to be retried. + // This situation is extremely likely to occur when the DSS is under concurrent load. + return stacktrace.Propagate(resErr, "Error in query: %s", deleteOperationQuery) + } + + if opID == "" || opID != id { + return stacktrace.NewError("Could not delete Operation that does not exist. Delete: %s, returned opID: %s", id, opID) } return nil