Skip to content

Commit

Permalink
Do not set an http 500 error response when retrying db tx
Browse files Browse the repository at this point in the history
Since we're now retrying delete transactions, setting the http 500
response in dbErrorResponse() should only happen once the maximum
number of retries has been reached.

This also adds some logging of the uuid when upsert operations start,
and makes some logging formatting more consistent.
  • Loading branch information
ScottGarman committed Jun 8, 2023
1 parent 7dcd7bf commit 936055d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
12 changes: 8 additions & 4 deletions internal/upserter/upserter.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ func UpsertMetadata(ctx context.Context, db *sqlx.DB, logger *zap.Logger, id str
return metadata.Upsert(c, exec, true, []string{"id"}, boil.Whitelist("metadata"), boil.Infer())
}

logger.Sugar().Info("Starting metadata upsert for uuid: ", id)

return doUpsert(ctx, db, logger, id, ipAddresses, metadataUpserter)
}

Expand All @@ -42,6 +44,8 @@ func UpsertUserdata(ctx context.Context, db *sqlx.DB, logger *zap.Logger, id str
return userdata.Upsert(c, exec, true, []string{"id"}, boil.Whitelist("userdata"), boil.Infer())
}

logger.Sugar().Info("Starting userdata upsert for uuid: ", id)

return doUpsert(ctx, db, logger, id, ipAddresses, userdataUpserter)
}

Expand Down Expand Up @@ -119,7 +123,7 @@ func doUpsert(ctx context.Context, db *sqlx.DB, logger *zap.Logger, id string, i
upsertSuccess = true

if i > 0 {
logger.Sugar().Info("DB upsert transaction for instance ", id, "successful on retry attempt #", i)
logger.Sugar().Info("DB upsert transaction for instance: ", id, " successful on retry attempt #", i)
}
} else {
// Exponential backoff would be overkill here, but adding a bit of jitter
Expand All @@ -130,7 +134,7 @@ func doUpsert(ctx context.Context, db *sqlx.DB, logger *zap.Logger, id string, i
}

if !upsertSuccess {
logger.Sugar().Error("Upsert operation failed for instance ", id, " even after ", maxUpsertRetries, " attempts")
logger.Sugar().Error("Upsert operation failed for instance: ", id, " even after ", maxUpsertRetries, " attempts")
return err
}

Expand All @@ -154,7 +158,7 @@ func performUpsert(ctx context.Context, db *sqlx.DB, logger *zap.Logger, id stri

err := tx.Rollback()
if err != nil {
logger.Sugar().Error("Could not roll back upserter transaction for instance ", id, "error", err)
logger.Sugar().Error("Could not roll back upserter transaction for instance: ", id, "Error: ", err)
}
}
}()
Expand Down Expand Up @@ -216,7 +220,7 @@ func performUpsert(ctx context.Context, db *sqlx.DB, logger *zap.Logger, id stri
if err != nil {
txErr = true

logger.Sugar().Warn("Unable to commit db upsert transaction: ", err)
logger.Sugar().Warn("Unable to commit db upsert transaction for instance: ", id, "Error: ", err)

return err
}
Expand Down
16 changes: 6 additions & 10 deletions pkg/api/v1/router_instance_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,7 @@ func performDeletions(c *gin.Context, r *Router, instanceID string, metadata *mo

tx, err := r.DB.BeginTx(c, nil)
if err != nil {
r.Logger.Sugar().Warn("Something went wrong when running DB.BeginTX() for instance: ", instanceID)
dbErrorResponse(r.Logger, c, err)
r.Logger.Sugar().Warn("Something went wrong when running DB.BeginTX() for instance: ", instanceID, err)

return err
}
Expand All @@ -438,7 +437,7 @@ func performDeletions(c *gin.Context, r *Router, instanceID string, metadata *mo

err := tx.Rollback()
if err != nil {
r.Logger.Sugar().Error("Could not rollback delete transaction for instance ", instanceID, "error", err)
r.Logger.Sugar().Error("Could not rollback delete transaction for instance: ", instanceID, "Error: ", err)
}
}
}()
Expand All @@ -450,8 +449,7 @@ func performDeletions(c *gin.Context, r *Router, instanceID string, metadata *mo
if err != nil {
txErr = true

r.Logger.Sugar().Warn("Something went wrong when setting up metadata.Delete transaction for instance: ", instanceID)
dbErrorResponse(r.Logger, c, err)
r.Logger.Sugar().Warn("Something went wrong when setting up metadata.Delete transaction for instance: ", instanceID, "Error: ", err)

return err
}
Expand All @@ -462,8 +460,7 @@ func performDeletions(c *gin.Context, r *Router, instanceID string, metadata *mo
if err != nil {
txErr = true

r.Logger.Sugar().Warn("Something went wrong when setting up userdata.Delete transaction for instance: ", instanceID)
dbErrorResponse(r.Logger, c, err)
r.Logger.Sugar().Warn("Something went wrong when setting up userdata.Delete transaction for instance: ", instanceID, "Error: ", err)

return err
}
Expand All @@ -477,8 +474,7 @@ func performDeletions(c *gin.Context, r *Router, instanceID string, metadata *mo
if err != nil {
txErr = true

r.Logger.Sugar().Warn("Something went wrong when setting up deleteInstanceIPs transaction for instance: ", instanceID)
dbErrorResponse(r.Logger, c, err)
r.Logger.Sugar().Warn("Something went wrong when setting up deleteInstanceIPs transaction for instance: ", instanceID, "Error: ", err)

return err
}
Expand All @@ -490,7 +486,7 @@ func performDeletions(c *gin.Context, r *Router, instanceID string, metadata *mo
if err != nil {
txErr = true

r.Logger.Sugar().Warn("Unable to commit db delete transaction: ", err)
r.Logger.Sugar().Warn("Unable to commit db delete transaction for instance: ", instanceID, "Error: ", err)

return err
}
Expand Down

0 comments on commit 936055d

Please sign in to comment.