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

Jump to bottom

fix tablegc flaky test #10230

Merged
merged 13 commits into from
May 9, 2022
Merged

fix tablegc flaky test #10230

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
539c265
fix tablegc flaky test
shlomi-noach May 8, 2022
d58999f
Pseudo GC state: TableDroppedGCState
shlomi-noach May 8, 2022
0e4cf35
allow next transition and any transition that follows; validate rowco…
shlomi-noach May 8, 2022
2b91f95
perform validation
shlomi-noach May 8, 2022
948f2cd
simplify
shlomi-noach May 8, 2022
f5f68a7
remove redundant check
shlomi-noach May 8, 2022
fde461d
more debug info
shlomi-noach May 8, 2022
136a8cf
reduce intervals
shlomi-noach May 8, 2022
ad7c758
do not waste time waiting
shlomi-noach May 8, 2022
cc05440
parameterize all time values
shlomi-noach May 9, 2022
09feae2
give the table more chance to get purged
shlomi-noach May 9, 2022
26a96f8
Merge branch 'main' into fix-tablegc-flaky-test
shlomi-noach May 9, 2022
ea62fde
flakiness: remove --heartbeat_on_demand_duration
shlomi-noach May 9, 2022
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
174 changes: 82 additions & 92 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 @@ -16,6 +16,7 @@ limitations under the License.
package tablegc

import (
"context"
"flag"
"os"
"testing"
Expand Down Expand Up @@ -90,8 +91,8 @@ func TestMain(m *testing.M) {
"--enable_replication_reporter",
"--heartbeat_enable",
"--heartbeat_interval", "250ms",
"--gc_check_interval", "5s",
"--gc_purge_check_interval", "5s",
"--gc_check_interval", "2s",
"--gc_purge_check_interval", "2s",
"--table_gc_lifecycle", "hold,purge,evac,drop",
}
// We do not need semiSync for this test case.
Expand Down Expand Up @@ -160,12 +161,73 @@ func tableExists(tableExpr string) (exists bool, tableName string, err error) {
return true, row.AsString("Name", ""), nil
}

// tableExists sees that a given table exists in MySQL
func dropTable(tableName string) (err error) {
func validateTableDoesNotExist(t *testing.T, tableExpr string) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

ticker := time.NewTicker(time.Second)
var foundTableName string
var exists bool
var err error
for {
select {
case <-ticker.C:
exists, foundTableName, err = tableExists(tableExpr)
require.NoError(t, err)
if !exists {
return
}
case <-ctx.Done():
assert.NoError(t, ctx.Err(), "validateTableDoesNotExist timed out, table %v still exists (%v)", tableExpr, foundTableName)
return
}
}
}

func validateAnyState(t *testing.T, expectNumRows int64, states ...schema.TableGCState) {
for _, state := range states {
expectTableToExist := true
searchExpr := ""
switch state {
case schema.HoldTableGCState:
searchExpr = `\_vt\_HOLD\_%`
case schema.PurgeTableGCState:
searchExpr = `\_vt\_PURGE\_%`
case schema.EvacTableGCState:
searchExpr = `\_vt\_EVAC\_%`
case schema.DropTableGCState:
searchExpr = `\_vt\_DROP\_%`
case schema.TableDroppedGCState:
searchExpr = `\_vt\_%`
expectTableToExist = false
default:
t.Log("Unknown state")
t.Fail()
}
exists, tableName, err := tableExists(searchExpr)
require.NoError(t, err)

if exists {
if expectNumRows >= 0 {
checkTableRows(t, tableName, expectNumRows)
}
// Now that the table is validated, we can drop it
dropTable(t, tableName)
}
if exists == expectTableToExist {
// condition met
return
}
}
assert.Fail(t, "could not match any of the states: %v", states)
}

// dropTable drops a table
func dropTable(t *testing.T, tableName string) {
query := `drop table if exists %a`
parsed := sqlparser.BuildParsedQuery(query, tableName)
_, err = primaryTablet.VttabletProcess.QueryTablet(parsed.Query, keyspaceName, true)
return err
_, err := primaryTablet.VttabletProcess.QueryTablet(parsed.Query, keyspaceName, true)
require.NoError(t, err)
}

func TestPopulateTable(t *testing.T) {
Expand All @@ -175,11 +237,7 @@ func TestPopulateTable(t *testing.T) {
assert.NoError(t, err)
assert.True(t, exists)
}
{
exists, _, err := tableExists("no_such_table")
assert.NoError(t, err)
assert.False(t, exists)
}
validateTableDoesNotExist(t, "no_such_table")
}

func TestHold(t *testing.T) {
Expand All @@ -190,11 +248,7 @@ func TestHold(t *testing.T) {
_, err = primaryTablet.VttabletProcess.QueryTablet(query, keyspaceName, true)
assert.NoError(t, err)

{
exists, _, err := tableExists("t1")
assert.NoError(t, err)
assert.False(t, exists)
}
validateTableDoesNotExist(t, "t1")
{
exists, _, err := tableExists(tableName)
assert.NoError(t, err)
Expand All @@ -214,18 +268,9 @@ func TestHold(t *testing.T) {
time.Sleep(10 * time.Second)
{
// We're now both beyond table's timestamp as well as a tableGC interval
exists, _, err := tableExists(tableName)
assert.NoError(t, err)
assert.False(t, exists)
}
{
// Table should be renamed as _vt_PURGE_...
exists, purgeTableName, err := tableExists(`\_vt\_PURGE\_%`)
assert.NoError(t, err)
assert.True(t, exists)
err = dropTable(purgeTableName)
assert.NoError(t, err)
validateTableDoesNotExist(t, tableName)
}
validateAnyState(t, -1, schema.PurgeTableGCState, schema.EvacTableGCState, schema.DropTableGCState, schema.TableDroppedGCState)
}

func TestEvac(t *testing.T) {
Expand All @@ -236,11 +281,7 @@ func TestEvac(t *testing.T) {
_, err = primaryTablet.VttabletProcess.QueryTablet(query, keyspaceName, true)
assert.NoError(t, err)

{
exists, _, err := tableExists("t1")
assert.NoError(t, err)
assert.False(t, exists)
}
validateTableDoesNotExist(t, "t1")
{
exists, _, err := tableExists(tableName)
assert.NoError(t, err)
Expand All @@ -258,19 +299,11 @@ func TestEvac(t *testing.T) {
}

time.Sleep(10 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but can we make this a variable ~ createTableTimeStampOffsetSecs and then use that everywhere we are doing time.Sleep in relation to that variable? We can then use this throughout:

time.Sleep(createTableTimeStampOffsetSecs * time.Second)
time.Sleep((createTableTimeStampOffsetSecs / 2) * time.Second)
time.Sleep((createTableTimeStampOffsetSecs * 3) * time.Second)
...

This way it's easier to tie all the timing behavior together and adjust it holistically as needed by increasing that one variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All time values now parameterized

{
// We're now both beyond table's timestamp as well as a tableGC interval
exists, _, err := tableExists(tableName)
assert.NoError(t, err)
assert.False(t, exists)
}
// We're now both beyond table's timestamp as well as a tableGC interval
validateTableDoesNotExist(t, tableName)
time.Sleep(5 * time.Second)
{
// Table should be renamed as _vt_DROP_... and then dropped!
exists, _, err := tableExists(`\_vt\_DROP\_%`)
assert.NoError(t, err)
assert.False(t, exists)
}
// Table should be renamed as _vt_DROP_... and then dropped!
validateAnyState(t, 0, schema.DropTableGCState, schema.TableDroppedGCState)
}

func TestDrop(t *testing.T) {
Expand All @@ -281,19 +314,11 @@ func TestDrop(t *testing.T) {
_, err = primaryTablet.VttabletProcess.QueryTablet(query, keyspaceName, true)
assert.NoError(t, err)

{
exists, _, err := tableExists("t1")
assert.NoError(t, err)
assert.False(t, exists)
}
validateTableDoesNotExist(t, "t1")

time.Sleep(20 * time.Second) // 10s for timestamp to pass, then 10s for checkTables and drop of table
{
// We're now both beyond table's timestamp as well as a tableGC interval
exists, _, err := tableExists(tableName)
assert.NoError(t, err)
assert.False(t, exists)
}
// We're now both beyond table's timestamp as well as a tableGC interval
validateTableDoesNotExist(t, tableName)
}

func TestPurge(t *testing.T) {
Expand Down Expand Up @@ -326,21 +351,7 @@ func TestPurge(t *testing.T) {
}

time.Sleep(15 * time.Second) // purgeReentranceInterval
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar nit here. We could have a variable called purgeReentranceInterval so that those vars are all together and it's clear/easy to adjust the waits.

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

{
// We're now both beyond table's timestamp as well as a tableGC interval
exists, _, err := tableExists(tableName)
require.NoError(t, err)
require.False(t, exists)
}
{
// Table should be renamed as _vt_EVAC_...
exists, evacTableName, err := tableExists(`\_vt\_EVAC\_%`)
require.NoError(t, err)
require.True(t, exists)
checkTableRows(t, evacTableName, 0)
err = dropTable(evacTableName)
require.NoError(t, err)
}
validateAnyState(t, 0, schema.EvacTableGCState, schema.DropTableGCState, schema.TableDroppedGCState)
}

func TestPurgeView(t *testing.T) {
Expand Down Expand Up @@ -392,26 +403,5 @@ func TestPurgeView(t *testing.T) {
require.NoError(t, err)
require.True(t, exists)
}
{
// View should be renamed as _vt_EVAC_ or _vt_DROP: views only spend a fraction of a second in "EVAC"
// because evacuation is irrelevant to views. They are immediately renamed to DROP.
// Because there might be a race condition, we allow both cases
evacTableExists, evacTableName, err := tableExists(`\_vt\_EVAC\_%`)
require.NoError(t, err)

dropTableExists, dropTableName, err := tableExists(`\_vt\_DROP\_%`)
require.NoError(t, err)

require.True(t, evacTableExists || dropTableExists)
switch {
case evacTableExists:
checkTableRows(t, evacTableName, 1024) // the renamed view still points to t1's data
err = dropTable(evacTableName)
require.NoError(t, err)
case dropTableExists:
checkTableRows(t, dropTableName, 1024) // the renamed view still points to t1's data
err = dropTable(dropTableName)
require.NoError(t, err)
}
}
validateAnyState(t, 1024, schema.EvacTableGCState, schema.DropTableGCState, schema.TableDroppedGCState)
}
2 changes: 2 additions & 0 deletions go/vt/schema/tablegc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ const (
EvacTableGCState TableGCState = "EVAC"
// DropTableGCState is the state where the table is to be dropped. Probably ASAP
DropTableGCState TableGCState = "DROP"
// TableDroppedGCState is a pseudo state; a hint that the table does not exists anymore
TableDroppedGCState TableGCState = ""
)

var (
Expand Down