Skip to content

Commit

Permalink
Nitting while waiting for reviews
Browse files Browse the repository at this point in the history 
Signed-off-by: Matt Lord <[email protected]>
  • Loading branch information
@mattlord
mattlord committed Dec 19, 2024
1 parent e68c035 commit 8ad1fc0
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 42 deletions.
8 changes: 4 additions & 4 deletions go/test/endtoend/vreplication/vreplication_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ func testVreplicationWorkflows(t *testing.T, limited bool, binlogRowImage string
if binlogRowImage != "" {
// Run the e2e test with binlog_row_image=NOBLOB and
// binlog_row_value_options=PARTIAL_JSON.
require.NoError(t, utils.SetBinlogRowImageMode("noblob", vc.ClusterConfig.tmpDir, true))
defer utils.SetBinlogRowImageMode("", vc.ClusterConfig.tmpDir, false)
require.NoError(t, utils.SetBinlogRowImageOptions("noblob", true, vc.ClusterConfig.tmpDir))
defer utils.SetBinlogRowImageOptions("", false, vc.ClusterConfig.tmpDir)
}

defaultCell := vc.Cells[defaultCellName]
Expand Down Expand Up @@ -604,8 +604,8 @@ func TestCellAliasVreplicationWorkflow(t *testing.T) {

// Run the e2e test with binlog_row_image=NOBLOB and
// binlog_row_value_options=PARTIAL_JSON.
require.NoError(t, utils.SetBinlogRowImageMode("noblob", vc.ClusterConfig.tmpDir, true))
defer utils.SetBinlogRowImageMode("", vc.ClusterConfig.tmpDir, false)
require.NoError(t, utils.SetBinlogRowImageOptions("noblob", true, vc.ClusterConfig.tmpDir))
defer utils.SetBinlogRowImageOptions("", false, vc.ClusterConfig.tmpDir)

cell1 := vc.Cells["zone1"]
cell2 := vc.Cells["zone2"]
Expand Down
6 changes: 3 additions & 3 deletions go/test/utils/binlog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ const (
BinlogRowImageCnf = "binlog-row-image.cnf"
)

// SetBinlogRowImageMode creates a temp cnf file to set binlog_row_image=NOBLOB and
// SetBinlogRowImageOptions creates a temp cnf file to set binlog_row_image=NOBLOB and
// optionally binlog_row_value_options=PARTIAL_JSON (since it does not exist in 5.7)
// for vreplication unit tests.
// It adds it to the EXTRA_MY_CNF environment variable which appends text from them
// into my.cnf.
func SetBinlogRowImageMode(mode string, cnfDir string, includePartialJSON bool) error {
func SetBinlogRowImageOptions(mode string, partialJSON bool, cnfDir string) error {
var newCnfs []string

// remove any existing extra cnfs for binlog row image
Expand All @@ -59,7 +59,7 @@ func SetBinlogRowImageMode(mode string, cnfDir string, includePartialJSON bool)
if err != nil {
return err
}
if includePartialJSON {
if partialJSON {
if !CIDBPlatformIsMySQL8orLater() {
return fmt.Errorf("partial JSON values are only supported in MySQL 8.0 or later")
}
Expand Down
10 changes: 5 additions & 5 deletions go/test/utils/binlog_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ import (
"github.com/stretchr/testify/require"
)

// TestSetBinlogRowImageMode tests the SetBinlogRowImageMode function.
// TestSetBinlogRowImageOptions tests the SetBinlogRowImageOptions function.
func TestUtils(t *testing.T) {
tmpDir := "/tmp"
cnfFile := fmt.Sprintf("%s/%s", tmpDir, BinlogRowImageCnf)

// Test that setting the mode will create the cnf file and add it to the EXTRA_MY_CNF env var.
require.NoError(t, SetBinlogRowImageMode("noblob", tmpDir, false))
require.NoError(t, SetBinlogRowImageOptions("noblob", false, tmpDir))
data, err := os.ReadFile(cnfFile)
require.NoError(t, err)
require.Contains(t, string(data), "binlog_row_image=noblob")
Expand All @@ -39,18 +39,18 @@ func TestUtils(t *testing.T) {
// Test that setting the mode and passing true for includePartialJSON will set both options
// as expected.
if CIDBPlatformIsMySQL8orLater() {
require.NoError(t, SetBinlogRowImageMode("noblob", tmpDir, true))
require.NoError(t, SetBinlogRowImageOptions("noblob", true, tmpDir))
data, err = os.ReadFile(cnfFile)
require.NoError(t, err)
require.Contains(t, string(data), "binlog_row_image=noblob")
require.Contains(t, string(data), "binlog_row_value_options=PARTIAL_JSON")
require.Contains(t, os.Getenv(ExtraCnf), BinlogRowImageCnf)
} else {
require.Error(t, SetBinlogRowImageMode("noblob", tmpDir, true))
require.Error(t, SetBinlogRowImageOptions("noblob", true, tmpDir))
}

// Test that clearing the mode will remove the cnf file and the cnf from the EXTRA_MY_CNF env var.
require.NoError(t, SetBinlogRowImageMode("", tmpDir, false))
require.NoError(t, SetBinlogRowImageOptions("", false, tmpDir))
require.NotContains(t, os.Getenv(ExtraCnf), BinlogRowImageCnf)
_, err = os.Stat(cnfFile)
require.True(t, os.IsNotExist(err))
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vttablet/tabletmanager/vreplication/framework_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ func TestMain(m *testing.M) {
exitCode := func() int {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
if err := utils.SetBinlogRowImageMode("full", tempDir, false); err != nil {
if err := utils.SetBinlogRowImageOptions("full", false, tempDir); err != nil {
panic(err)
}
defer utils.SetBinlogRowImageMode("", tempDir, false)
defer utils.SetBinlogRowImageOptions("", false, tempDir)
cancel, ret := setup(ctx)
if ret > 0 {
return ret
Expand All @@ -202,10 +202,10 @@ func TestMain(m *testing.M) {
// We still run unit tests with MySQL 5.7, so we cannot add it to the cnf file
// when using 5.7 or mysqld will fail to start.
runPartialJSONTest = utils.CIDBPlatformIsMySQL8orLater()
if err := utils.SetBinlogRowImageMode("noblob", tempDir, runPartialJSONTest); err != nil {
if err := utils.SetBinlogRowImageOptions("noblob", runPartialJSONTest, tempDir); err != nil {
panic(err)
}
defer utils.SetBinlogRowImageMode("", tempDir, false)
defer utils.SetBinlogRowImageOptions("", false, tempDir)
cancel, ret = setup(ctx)
if ret > 0 {
return ret
Expand Down
18 changes: 8 additions & 10 deletions go/vt/vttablet/tabletmanager/vreplication/replicator_plan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,11 @@ func (tp *TablePlan) applyChange(rowChange *binlogdatapb.RowChange, executor fun
// This occurs when using partial JSON values as a result of mysqld using
// binlog-row-value-options=PARTIAL_JSON.
if len(afterVals[i].Raw()) == 0 {
// When using BOTH binlog_row_image=NOBLOB AND
// binlog_row_value_options=PARTIAL_JSON, if the JSON column was NOT updated
// then the JSON column is marked as partial and the diff is empty as a way
// to exclude it from the AFTER image. It still has the data bit set, however,
// even though it's not really present. So we have to account for this by
// unsetting the data bit so that the current JSON value is not lost.
// If the JSON column was NOT updated then the JSON column is marked as
// partial and the diff is empty as a way to exclude it from the AFTER image.
// It still has the data bit set, however, even though it's not really
// present. So we have to account for this by unsetting the data bit so
// that the column's current JSON value is not lost.
setBit(rowChange.DataColumns.Cols, i, false)
newVal = ptr.Of(sqltypes.MakeTrusted(querypb.Type_EXPRESSION, nil))
} else {
Expand Down Expand Up @@ -486,10 +485,9 @@ func (tp *TablePlan) applyChange(rowChange *binlogdatapb.RowChange, executor fun
case !isBitSet(rowChange.JsonPartialValues.Cols, jsonIndex):
// We use the full AFTER value which we already have.
case len(afterVals[i].Raw()) == 0:
// When using BOTH binlog_row_image=NOBLOB AND
// binlog_row_value_options=PARTIAL_JSON, if the JSON column was NOT updated
// then the JSON column is marked as partial and the diff is empty as a way
// to exclude it from the AFTER image. So we want to use the BEFORE image value.
// If the JSON column was NOT updated then the JSON column is marked as partial
// and the diff is empty as a way to exclude it from the AFTER image. So we
// want to use the BEFORE image value.
beforeVal, err := vjson.MarshalSQLValue(bindvars["b_"+field.Name].Value)
if err != nil {
return nil, vterrors.Wrapf(err, "failed to convert JSON to SQL field value for %s.%s when building insert query",
Expand Down
53 changes: 37 additions & 16 deletions go/vt/vttablet/tabletmanager/vreplication/vplayer_flaky_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1521,19 +1521,11 @@ func TestPlayerRowMove(t *testing.T) {

// TestPlayerPartialImagesUpdatePK tests the behavior of the vplayer when
// updating the Primary Key for a row when we have partial binlog
// images, meaning that binlog-row-image=NOBLOB and
// binlog-row-value-options=PARTIAL_JSON. These are both set when running
// the unit tests with runNoBlobTest=true and runPartialJSONTest=true.
// If the test is running in the CI and the database platform is not MySQL
// 8.0 or later (which you can control using the CI_DB_PLATFORM env
// variable), then runPartialJSONTest will be false and the test will be
// skipped.
// images, meaning that binlog-row-image=NOBLOB and/or
// binlog-row-value-options=PARTIAL_JSON.
func TestPlayerPartialImagesUpdatePK(t *testing.T) {
if !runNoBlobTest {
t.Skip("Skipping test as binlog_row_image=NOBLOB is not set")
}
if !runPartialJSONTest {
t.Skip("Skipping test as binlog-row-value-options=PARTIAL_JSON is not set (most likely because the database type is not MySQL 8.0 or later)")
t.Skip("Skipping test as binlog_row_value_options=PARTIAL_JSON is not enabled")
}

defer deleteTablet(addTablet(100))
Expand Down Expand Up @@ -1577,15 +1569,29 @@ func TestPlayerPartialImagesUpdatePK(t *testing.T) {
{"3", "{\"key3\": \"val3\"}", "blob data3"},
},
},
{
}
if runNoBlobTest {
testCases = append(testCases, testCase{
input: `update src set jd=JSON_SET(jd, '$.color', 'red') where id = 1`,
output: []string{"update dst set jd=JSON_INSERT(`jd`, _utf8mb4'$.color', _utf8mb4\"red\") where id=1"},
data: [][]string{
{"1", "{\"key1\": \"val1\", \"color\": \"red\"}", "blob data"},
{"2", "{\"key2\": \"val2\"}", "blob data2"},
{"3", "{\"key3\": \"val3\"}", "blob data3"},
},
},
})
} else {
testCases = append(testCases, testCase{
input: `update src set jd=JSON_SET(jd, '$.color', 'red') where id = 1`,
output: []string{"update dst set jd=JSON_INSERT(`jd`, _utf8mb4'$.color', _utf8mb4\"red\"), bd=_binary'blob data' where id=1"},
data: [][]string{
{"1", "{\"key1\": \"val1\", \"color\": \"red\"}", "blob data"},
{"2", "{\"key2\": \"val2\"}", "blob data2"},
{"3", "{\"key3\": \"val3\"}", "blob data3"},
},
})
}
testCases = append(testCases, []testCase{
{
input: `update src set bd = 'new blob data' where id = 2`,
output: []string{
Expand All @@ -1609,10 +1615,25 @@ func TestPlayerPartialImagesUpdatePK(t *testing.T) {
{"12", "{\"key2\": \"val2\"}", "newest blob data"},
},
},
{
}...)
if runNoBlobTest {
testCases = append(testCases, testCase{
input: `update src set id = id+10 where id = 3`,
error: "binary log event missing a needed value for dst.bd due to not using binlog-row-image=FULL",
},
})
} else {
testCases = append(testCases, testCase{
input: `update src set id = id+10 where id = 3`,
output: []string{
"delete from dst where id=3",
"insert into dst(id,jd,bd) values (13,JSON_OBJECT(_utf8mb4'key3', _utf8mb4'val3'),_binary'blob data3')",
},
data: [][]string{
{"1", "{\"key1\": \"val1\", \"color\": \"red\"}", "blob data"},
{"12", "{\"key2\": \"val2\"}", "newest blob data"},
{"13", "{\"key3\": \"val3\"}", "blob data3"},
},
})
}

for _, tc := range testCases {
Expand Down Expand Up @@ -1776,7 +1797,7 @@ func TestPlayerTypes(t *testing.T) {
{"2", "null", `{"name": null}`, "123", `{"a": [42, 100]}`, `{"foo": "bar"}`},
},
}}
if runNoBlobTest && runPartialJSONTest {
if runPartialJSONTest {
// With partial JSON values we don't replicate the JSON columns that aren't
// actually updated.
testcases = append(testcases, testcase{
Expand Down

0 comments on commit 8ad1fc0

Please sign in to comment.