Skip to content

Commit

Permalink
Merge pull request #350 from cashapp/prudhvi.mdl_lock_name_fix
Browse files Browse the repository at this point in the history
Use tablename for metadatalock name as tablename respsects both the m…
  • Loading branch information
morgo authored Sep 27, 2024
2 parents 77598d0 + c5d093c commit c396803
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/migration/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func (r *Runner) Run(originalCtx context.Context) error {
}

// Take a metadata lock to prevent other migrations from running concurrently.
r.metadataLock, err = dbconn.NewMetadataLock(ctx, r.dsn(), r.table.QuotedName, r.logger)
r.metadataLock, err = dbconn.NewMetadataLock(ctx, r.dsn(), r.table.TableName, r.logger)
if err != nil {
return err
}
Expand Down
10 changes: 8 additions & 2 deletions pkg/migration/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func TestTableLength(t *testing.T) {
assert.NoError(t, err)
err = m.Run(context.Background())
assert.Error(t, err)
assert.ErrorContains(t, err, "is too long")
assert.ErrorContains(t, err, "table name must be less than 54 characters")
assert.NoError(t, m.Close())

// There is another condition where the error will be in dropping the _old table first
Expand All @@ -350,7 +350,7 @@ func TestTableLength(t *testing.T) {
assert.NoError(t, err)
err = m.Run(context.Background())
assert.Error(t, err)
assert.ErrorContains(t, err, "is too long")
assert.ErrorContains(t, err, "table name must be less than 54 characters")
assert.NoError(t, m.Close())
}

Expand Down Expand Up @@ -2763,6 +2763,12 @@ func TestResumeFromCheckpointE2EWithManualSentinel(t *testing.T) {
continue // table does not exist yet
}
if rowCount > 0 {
// Test that it's not possible to acquire metadata lock with name
// as tablename while the migration is running.
lock, err := dbconn.NewMetadataLock(ctx, testutils.DSN(),
tableName, &testLogger{})
assert.ErrorContains(t, err, "could not acquire metadata lock: resume_checkpoint_e2e_w_sentinel, lock is held by another connection")
assert.Nil(t, lock)
break
}
}
Expand Down

0 comments on commit c396803

Please sign in to comment.