Skip to content
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

New unified internal table names format: part 2, generating new names #15178

Merged
merged 7 commits into from
Feb 15, 2024
Merged

New unified internal table names format: part 2, generating new names #15178

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
078bb0f
New unified internal table names format: part 2, generating new names
shlomi-noach Feb 8, 2024
f78e48f
restore functions used in endtoend test
shlomi-noach Feb 11, 2024
56db8cf
check error code
shlomi-noach Feb 11, 2024
4ca4459
use ReadableTimestamp()
shlomi-noach Feb 11, 2024
34438a0
more tests
shlomi-noach Feb 11, 2024
f720936
typo
shlomi-noach Feb 11, 2024
ec2b976
states to hint: make it a map
shlomi-noach Feb 13, 2024
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
6 changes: 3 additions & 3 deletions go/test/endtoend/tabletmanager/tablegc/tablegc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ func TestPopulateTable(t *testing.T) {

func generateRenameStatement(newFormat bool, fromTableName string, state schema.TableGCState, tm time.Time) (statement string, toTableName string, err error) {
if newFormat {
return schema.GenerateRenameStatementNewFormat(fromTableName, state, tm)
return schema.GenerateRenameStatement(fromTableName, state, tm)
}
return schema.GenerateRenameStatement(fromTableName, state, tm)
return schema.GenerateRenameStatementOldFormat(fromTableName, state, tm)
}

func TestHold(t *testing.T) {
Expand Down Expand Up @@ -448,7 +448,7 @@ func TestPurge(t *testing.T) {

func TestPurgeView(t *testing.T) {
populateTable(t)
query, tableName, err := schema.GenerateRenameStatement("v1", schema.PurgeTableGCState, time.Now().UTC().Add(tableTransitionExpiration))
query, tableName, err := generateRenameStatement(true, "v1", schema.PurgeTableGCState, time.Now().UTC().Add(tableTransitionExpiration))
require.NoError(t, err)

_, err = primaryTablet.VttabletProcess.QueryTablet(query, keyspaceName, true)
Expand Down
56 changes: 55 additions & 1 deletion go/vt/schema/name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package schema

import (
"fmt"
"regexp"
"strings"
"time"
Expand All @@ -32,8 +33,23 @@
InternalTableNameExpression string = `^_vt_([a-zA-Z0-9]{3})_([0-f]{32})_([0-9]{14})_$`
)

type InternalTableHint string

const (
InternalTableUnknownHint InternalTableHint = "nil"
InternalTableGCHoldHint InternalTableHint = "hld"
InternalTableGCPurgeHint InternalTableHint = "prg"
InternalTableGCEvacHint InternalTableHint = "evc"
InternalTableGCDropHint InternalTableHint = "drp"
InternalTableVreplicationHint InternalTableHint = "vrp"
)

func (h InternalTableHint) String() string {
return string(h)
}

var (
// internalTableNameRegexp parses new intrnal table name format, e.g. _vt_hld_6ace8bcef73211ea87e9f875a4d24e90_20200915120410_
// internalTableNameRegexp parses new internal table name format, e.g. _vt_hld_6ace8bcef73211ea87e9f875a4d24e90_20200915120410_
internalTableNameRegexp = regexp.MustCompile(InternalTableNameExpression)
)

Expand Down Expand Up @@ -65,6 +81,44 @@
return t.Format(readableTimeFormat)
}

// ReadableTimestamp returns the current timestamp, in seconds resolution, that is human readable
func ReadableTimestamp() string {
return ToReadableTimestamp(time.Now())

Check warning on line 86 in go/vt/schema/name.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schema/name.go#L85-L86 

Added lines #L85 - L86 were not covered by tests
}

func condenseUUID(uuid string) string {
uuid = strings.ReplaceAll(uuid, "-", "")
uuid = strings.ReplaceAll(uuid, "_", "")
return uuid
}

// isCondensedUUID answers 'true' when the given string is a condensed UUID, e.g.:
// a0638f6bec7b11ea9bf8000d3a9b8a9a
func isCondensedUUID(uuid string) bool {
return condensedUUIDRegexp.MatchString(uuid)
}

// generateGCTableName creates an internal table name, based on desired hint and time, and with optional preset UUID.
// If uuid is given, then it must be in condensed-UUID format. If empty, the function auto-generates a UUID.
func GenerateInternalTableName(hint string, uuid string, t time.Time) (tableName string, err error) {
if len(hint) != 3 {
return "", fmt.Errorf("Invalid hint: %s, expected 3 characters", hint)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to use vterrors here so that we have codes? IIRC you added some error handling not too long ago that relied on error codes. Error messages also aren't supposed to be capitalized (for wrapping).

Copy link
Contributor

Choose a reason for hiding this comment

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

https://go.dev/wiki/CodeReviewComments#error-strings

}
if uuid == "" {
uuid, err = CreateUUIDWithDelimiter("")
} else {
uuid = condenseUUID(uuid)
}
if err != nil {
return "", err

Check warning on line 113 in go/vt/schema/name.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schema/name.go#L113 

Added line #L113 was not covered by tests
}
if !isCondensedUUID(uuid) {
return "", fmt.Errorf("Invalid UUID: %s, expected condensed 32 hexadecimals", uuid)
}
timestamp := ToReadableTimestamp(t)
return fmt.Sprintf("_vt_%s_%s_%s_", hint, uuid, timestamp), nil
}

// IsInternalOperationTableName answers 'true' when the given table name stands for an internal Vitess
// table used for operations such as:
// - Online DDL (gh-ost, pt-online-schema-change)
Expand Down
54 changes: 53 additions & 1 deletion go/vt/schema/name_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestNameIsGCTableName(t *testing.T) {
Expand Down Expand Up @@ -165,10 +166,61 @@ func TestAnalyzeInternalTableName(t *testing.T) {
assert.Equal(t, ts.isInternal, isInternal)
if ts.isInternal {
assert.NoError(t, err)
assert.True(t, IsGCUUID(uuid))
assert.True(t, isCondensedUUID(uuid))
assert.Equal(t, ts.hint, hint)
assert.Equal(t, ts.t, tm)
}
})
}
}

func TestToReadableTimestamp(t *testing.T) {
ti, err := time.Parse(time.UnixDate, "Wed Feb 25 11:06:39 PST 2015")
assert.NoError(t, err)

readableTimestamp := ToReadableTimestamp(ti)
assert.Equal(t, readableTimestamp, "20150225110639")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more meaningful if we didn't use a string literal here but instead used ti here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point was to validate that a transformation of ti lands at an expected value. So that constant should remain IMO. I did, however, switch between readableTimestamp and "20150225110639" because the constant is the expected one.

}

func TestGenerateInternalTableName(t *testing.T) {
ti, err := time.Parse(time.UnixDate, "Wed Feb 25 11:06:39 PST 2015")
assert.NoError(t, err)

{
uuid := "6ace8bcef73211ea87e9f875a4d24e90"
tableName, err := GenerateInternalTableName(InternalTableGCPurgeHint.String(), uuid, ti)
require.NoError(t, err)
assert.Equal(t, "_vt_prg_6ace8bcef73211ea87e9f875a4d24e90_20150225110639_", tableName)
assert.True(t, IsInternalOperationTableName(tableName))
}
{
uuid := "4e5dcf80_354b_11eb_82cd_f875a4d24e90"
tableName, err := GenerateInternalTableName(InternalTableGCPurgeHint.String(), uuid, ti)
require.NoError(t, err)
assert.Equal(t, "_vt_prg_4e5dcf80354b11eb82cdf875a4d24e90_20150225110639_", tableName)
assert.True(t, IsInternalOperationTableName(tableName))
}
{
uuid := "4e5dcf80-354b-11eb-82cd-f875a4d24e90"
tableName, err := GenerateInternalTableName(InternalTableGCPurgeHint.String(), uuid, ti)
require.NoError(t, err)
assert.Equal(t, "_vt_prg_4e5dcf80354b11eb82cdf875a4d24e90_20150225110639_", tableName)
assert.True(t, IsInternalOperationTableName(tableName))
}
{
uuid := ""
tableName, err := GenerateInternalTableName(InternalTableGCPurgeHint.String(), uuid, ti)
require.NoError(t, err)
assert.True(t, IsInternalOperationTableName(tableName))
}
{
uuid := "4e5dcf80_354b_11eb_82cd_f875a4d24e90_00001111"
_, err := GenerateInternalTableName(InternalTableGCPurgeHint.String(), uuid, ti)
require.ErrorContains(t, err, "Invalid UUID")
}
{
uuid := "6ace8bcef73211ea87e9f875a4d24e90"
_, err := GenerateInternalTableName("abcdefg", uuid, ti)
require.ErrorContains(t, err, "Invalid hint")
}
}
2 changes: 1 addition & 1 deletion go/vt/schema/online_ddl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestGetGCUUID(t *testing.T) {
onlineDDL, err := NewOnlineDDL("ks", "tbl", "alter table t drop column c", NewDDLStrategySetting(DDLStrategyDirect, ""), "", "", parser)
assert.NoError(t, err)
gcUUID := onlineDDL.GetGCUUID()
assert.True(t, IsGCUUID(gcUUID))
assert.True(t, isCondensedUUID(gcUUID))
uuids[gcUUID] = true
}
assert.Equal(t, count, len(uuids))
Expand Down
85 changes: 38 additions & 47 deletions go/vt/schema/tablegc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,85 +44,76 @@
TableDroppedGCState TableGCState = ""
)

func (s TableGCState) TableHint() InternalTableHint {
switch s {
case HoldTableGCState:
return InternalTableGCHoldHint
case PurgeTableGCState:
return InternalTableGCPurgeHint
case EvacTableGCState:
return InternalTableGCEvacHint
case DropTableGCState:
return InternalTableGCDropHint
default:
return InternalTableUnknownHint

Check warning on line 58 in go/vt/schema/tablegc.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schema/tablegc.go#L57-L58 

Added lines #L57 - L58 were not covered by tests
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to make a map of these states like protobuf does? Not a big deal, but the use of Internal (capital I) and Hint here are not obvious to the average reader (me).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

}

const (
GCTableNameExpression string = `^_vt_(HOLD|PURGE|EVAC|DROP)_([0-f]{32})_([0-9]{14})$`
// NewGCTableNameExpression parses new intrnal table name format, e.g. _vt_hld_6ace8bcef73211ea87e9f875a4d24e90_20200915120410_
NewGCTableNameExpression string = `^_vt_(hld|prg|evc|drp)_([0-f]{32})_([0-9]{14})_$`
OldGCTableNameExpression string = `^_vt_(HOLD|PURGE|EVAC|DROP)_([0-f]{32})_([0-9]{14})$`
// GCTableNameExpression parses new internal table name format, e.g. _vt_hld_6ace8bcef73211ea87e9f875a4d24e90_20200915120410_
GCTableNameExpression string = `^_vt_(hld|prg|evc|drp)_([0-f]{32})_([0-9]{14})_$`
)

var (
gcUUIDRegexp = regexp.MustCompile(`^[0-f]{32}$`)
gcTableNameRegexp = regexp.MustCompile(GCTableNameExpression)

gcStates = map[string]TableGCState{
string(HoldTableGCState): HoldTableGCState,
"hld": HoldTableGCState,
string(PurgeTableGCState): PurgeTableGCState,
"prg": PurgeTableGCState,
string(EvacTableGCState): EvacTableGCState,
"evc": EvacTableGCState,
string(DropTableGCState): DropTableGCState,
"drp": DropTableGCState,
}
condensedUUIDRegexp = regexp.MustCompile(`^[0-f]{32}$`)
oldGCTableNameRegexp = regexp.MustCompile(OldGCTableNameExpression)

gcStates = map[string]TableGCState{}
)

// IsGCUUID answers 'true' when the given string is an GC UUID, e.g.:
// a0638f6bec7b11ea9bf8000d3a9b8a9a
func IsGCUUID(uuid string) bool {
return gcUUIDRegexp.MatchString(uuid)
func init() {
for _, gcState := range []TableGCState{HoldTableGCState, PurgeTableGCState, EvacTableGCState, DropTableGCState} {
gcStates[string(gcState)] = gcState
gcStates[gcState.TableHint().String()] = gcState
}
}

// generateGCTableName creates a GC table name, based on desired state and time, and with optional preset UUID.
// If uuid is given, then it must be in GC-UUID format. If empty, the function auto-generates a UUID.
func generateGCTableName(state TableGCState, uuid string, t time.Time) (tableName string, err error) {
func generateGCTableNameOldFormat(state TableGCState, uuid string, t time.Time) (tableName string, err error) {
if uuid == "" {
uuid, err = CreateUUIDWithDelimiter("")
}
if err != nil {
return "", err
}
if !IsGCUUID(uuid) {
if !isCondensedUUID(uuid) {
return "", fmt.Errorf("Not a valid GC UUID format: %s", uuid)
}
timestamp := ToReadableTimestamp(t)
return fmt.Sprintf("_vt_%s_%s_%s", state, uuid, timestamp), nil
}

// generateGCTableNameNewFormat creates a GC table name, based on desired state and time, and with optional preset UUID.
// generateGCTableName creates a GC table name, based on desired state and time, and with optional preset UUID.
// If uuid is given, then it must be in GC-UUID format. If empty, the function auto-generates a UUID.
func generateGCTableNameNewFormat(state TableGCState, uuid string, t time.Time) (tableName string, err error) {
if uuid == "" {
uuid, err = CreateUUIDWithDelimiter("")
}
if err != nil {
return "", err
}
if !IsGCUUID(uuid) {
return "", fmt.Errorf("Not a valid GC UUID format: %s", uuid)
}
timestamp := ToReadableTimestamp(t)
var hint string
func generateGCTableName(state TableGCState, uuid string, t time.Time) (tableName string, err error) {
for k, v := range gcStates {
if v != state {
continue
}
if len(k) == 3 && k != string(state) { // the "new" format
hint = k
return GenerateInternalTableName(k, uuid, t)
}
}
return fmt.Sprintf("_vt_%s_%s_%s_", hint, uuid, timestamp), nil
return "", fmt.Errorf("Unknown GC state: %v", state)

Check warning on line 109 in go/vt/schema/tablegc.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schema/tablegc.go#L109 

Added line #L109 was not covered by tests
}

// GenerateGCTableName creates a GC table name, based on desired state and time, and with random UUID
func GenerateGCTableName(state TableGCState, t time.Time) (tableName string, err error) {
return generateGCTableName(state, "", t)
}

// GenerateGCTableNameNewFormat creates a GC table name, based on desired state and time, and with random UUID
func GenerateGCTableNameNewFormat(state TableGCState, t time.Time) (tableName string, err error) {
return generateGCTableNameNewFormat(state, "", t)
}

// AnalyzeGCTableName analyzes a given table name to see if it's a GC table, and if so, parse out
// its state, uuid, and timestamp
func AnalyzeGCTableName(tableName string) (isGCTable bool, state TableGCState, uuid string, t time.Time, err error) {
Expand All @@ -134,7 +125,7 @@
}
// Try old naming formats. These names will not be generated in v20.
// TODO(shlomi): the code below should be remvoed in v21
submatch := gcTableNameRegexp.FindStringSubmatch(tableName)
submatch := oldGCTableNameRegexp.FindStringSubmatch(tableName)
if len(submatch) == 0 {
return false, state, uuid, t, nil
}
Expand Down Expand Up @@ -165,8 +156,8 @@
}

// GenerateRenameStatementWithUUIDNewFormat generates a "RENAME TABLE" statement, where a table is renamed to a GC table, with preset UUID
func GenerateRenameStatementWithUUIDNewFormat(fromTableName string, state TableGCState, uuid string, t time.Time) (statement string, toTableName string, err error) {
toTableName, err = generateGCTableNameNewFormat(state, uuid, t)
func generateRenameStatementWithUUIDOldFormat(fromTableName string, state TableGCState, uuid string, t time.Time) (statement string, toTableName string, err error) {
toTableName, err = generateGCTableNameOldFormat(state, uuid, t)

Check warning on line 160 in go/vt/schema/tablegc.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schema/tablegc.go#L159-L160 

Added lines #L159 - L160 were not covered by tests
if err != nil {
return "", "", err
}
Expand All @@ -179,8 +170,8 @@
}

// GenerateRenameStatement generates a "RENAME TABLE" statement, where a table is renamed to a GC table.
func GenerateRenameStatementNewFormat(fromTableName string, state TableGCState, t time.Time) (statement string, toTableName string, err error) {
return GenerateRenameStatementWithUUIDNewFormat(fromTableName, state, "", t)
func GenerateRenameStatementOldFormat(fromTableName string, state TableGCState, t time.Time) (statement string, toTableName string, err error) {
return generateRenameStatementWithUUIDOldFormat(fromTableName, state, "", t)

Check warning on line 174 in go/vt/schema/tablegc.go

View check run for this annotation

Codecov / codecov/patch

go/vt/schema/tablegc.go#L173-L174 

Added lines #L173 - L174 were not covered by tests
}

// ParseGCLifecycle parses a comma separated list of gc states and returns a map of indicated states
Expand Down
29 changes: 23 additions & 6 deletions go/vt/schema/tablegc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,37 @@ import (
"github.com/stretchr/testify/require"
)

func TestGCStates(t *testing.T) {
// These are all hard coded
require.Equal(t, HoldTableGCState, gcStates["hld"])
require.Equal(t, HoldTableGCState, gcStates["HOLD"])
require.Equal(t, PurgeTableGCState, gcStates["prg"])
require.Equal(t, PurgeTableGCState, gcStates["PURGE"])
require.Equal(t, EvacTableGCState, gcStates["evc"])
require.Equal(t, EvacTableGCState, gcStates["EVAC"])
require.Equal(t, DropTableGCState, gcStates["drp"])
require.Equal(t, DropTableGCState, gcStates["DROP"])
_, ok := gcStates["purge"]
require.False(t, ok)
_, ok = gcStates["vrp"]
require.False(t, ok)
require.Equal(t, 2*4, len(gcStates)) // 4 states, 2 forms each
}

func TestIsGCTableName(t *testing.T) {
tm := time.Now()
states := []TableGCState{HoldTableGCState, PurgeTableGCState, EvacTableGCState, DropTableGCState}
for _, state := range states {
for i := 0; i < 10; i++ {
tableName, err := generateGCTableName(state, "", tm)
tableName, err := generateGCTableNameOldFormat(state, "", tm)
assert.NoError(t, err)
assert.True(t, IsGCTableName(tableName))
assert.Truef(t, IsGCTableName(tableName), "table name: %s", tableName)

tableName, err = generateGCTableNameNewFormat(state, "6ace8bcef73211ea87e9f875a4d24e90", tm)
tableName, err = generateGCTableName(state, "6ace8bcef73211ea87e9f875a4d24e90", tm)
assert.NoError(t, err)
assert.Truef(t, IsGCTableName(tableName), "table name: %s", tableName)

tableName, err = GenerateGCTableNameNewFormat(state, tm)
tableName, err = GenerateGCTableName(state, tm)
assert.NoError(t, err)
assert.Truef(t, IsGCTableName(tableName), "table name: %s", tableName)
}
Expand Down Expand Up @@ -77,7 +94,7 @@ func TestIsGCTableName(t *testing.T) {
t.Run("explicit regexp", func(t *testing.T) {
// NewGCTableNameExpression regexp is used externally by vreplication. Its a redundant form of
// InternalTableNameExpression, but is nonetheless required. We verify it works correctly
re := regexp.MustCompile(NewGCTableNameExpression)
re := regexp.MustCompile(GCTableNameExpression)
t.Run("accept", func(t *testing.T) {
names := []string{
"_vt_hld_6ace8bcef73211ea87e9f875a4d24e90_20200915120410_",
Expand Down Expand Up @@ -173,7 +190,7 @@ func TestAnalyzeGCTableName(t *testing.T) {
assert.Equal(t, ts.isGC, isGC)
if ts.isGC {
assert.NoError(t, err)
assert.True(t, IsGCUUID(uuid))
assert.True(t, isCondensedUUID(uuid))
assert.Equal(t, ts.state, state)
assert.Equal(t, ts.t, tm)
}
Expand Down
Loading
Loading