Skip to content

Commit

Permalink
fix(spanner): ensure failure of column_metadata table update doesn't …
Browse files Browse the repository at this point in the history
…cause an apply failure
  • Loading branch information
newtonnthiga committed Aug 22, 2024
1 parent 90a23ad commit 4ead0f3
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 3 deletions.
7 changes: 5 additions & 2 deletions internal/spanner/services/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -1127,7 +1127,9 @@ func (s *SpannerService) CreateSpannerTable(ctx context.Context, parent string,
}

if err := UpdateColumnMetadata(db, tableId, table.Schema.Columns); err != nil {
return nil, status.Errorf(codes.Internal, "Error updating column metadata table: %v", err)
// This is not a fatal error, so we log it and continue
tfLogger.Error(ctx, fmt.Sprintf("Error updating column metadata table: %v", err))
//return nil, status.Errorf(codes.Internal, "Error updating column metadata table: %v", err)
}

// Get created table
Expand Down Expand Up @@ -1726,7 +1728,8 @@ func (s *SpannerService) UpdateSpannerTable(ctx context.Context, table *SpannerT
}

if err := UpdateColumnMetadata(db, tableId, table.Schema.Columns); err != nil {
return nil, status.Errorf(codes.Internal, "Error updating column metadata table: %v", err)
// This is not a fatal error, so we log it and continue
tfLogger.Error(ctx, fmt.Sprintf("Error updating column metadata table: %v", err))
}

return table, nil
Expand Down
12 changes: 11 additions & 1 deletion internal/spanner/services/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,17 @@ func GetColumnMetadata(db *gorm.DB, tableName string) ([]*ColumnMetadata, error)
}
func UpdateColumnMetadata(db *gorm.DB, tableName string, columns []*SpannerTableColumn) error {
// Create or Update ColumnMetadata table
if err := db.AutoMigrate(&ColumnMetadata{}); err != nil {
// IMPORTANT: When tables don't depend on each other, terraform will attempt to create them in parallel.
// This can cause the migration to run at the same time for multiple tables, which can lead to a duplicate table error.
// To prevent this, we'll retry the migration a few times.
_, err := utils.Retry(5, 10*time.Second, func() (interface{}, error) {
if err := db.AutoMigrate(&ColumnMetadata{}); err != nil {
return nil, err
}

return nil, nil
})
if err != nil {
return err
}

Expand Down
44 changes: 44 additions & 0 deletions internal/utils/retry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package utils

import (
"math/rand"
"time"
)

// Retry is a utility function to retry a function a number of times with exponential backoff
// and jitter. It will return the result of the function if it succeeds, or the last error if
// it fails.
//
// If the error is a Stop, it will return the original error for later checking.
func Retry[R interface{}](attempts int, initialSleep time.Duration, f func() (R, error)) (R, error) {
if res, err := f(); err != nil {
if s, ok := err.(Stop); ok {
// Return the original error for later checking
return res, s.error
}

if attempts--; attempts > 0 {
// Calculate exponential backoff
sleep := initialSleep * (1 << uint(attempts))

// Add some randomness to prevent creating a Thundering Herd
jitter := time.Duration(rand.Int63n(int64(sleep)))
sleep = sleep + jitter/2

time.Sleep(sleep)
return Retry[R](attempts, initialSleep, f)
}
return res, err
} else {
return res, nil
}
}

type Stop struct {
error
}

// NonRetryableError is a utility function to return an error that will not be retried
func NonRetryableError(err error) Stop {
return Stop{err}
}
51 changes: 51 additions & 0 deletions internal/utils/retry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package utils

import (
"fmt"
"reflect"
"testing"
"time"
)

func TestRetry(t *testing.T) {
type args[R interface{}] struct {
attempts int
initialSleep time.Duration
f func() (R, error)
}
type testCase[R interface{}] struct {
name string
args args[R]
want R
wantErr bool
}
tests := []testCase[interface{}]{
{
name: "Test Retry",
args: args[interface{}]{
attempts: 2,
initialSleep: 5 * time.Second,
f: func() (interface{}, error) {
return "test", fmt.Errorf("error")
},
},
want: "test",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tStart := time.Now()
got, err := Retry(tt.args.attempts, tt.args.initialSleep, tt.args.f)
tEnd := time.Now()
fmt.Println("Time taken: ", tEnd.Sub(tStart))
if (err != nil) != tt.wantErr {
t.Errorf("Retry() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Retry() got = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 4ead0f3

Please sign in to comment.