Skip to content

Commit

Permalink
docker: disable binlog in new MySQL 8 containers
Browse files Browse the repository at this point in the history
Unlike other flavors, MySQL 8 servers normally have their binary log enabled
by default. This makes sense in production, but it is undesirable for
ephemeral containerized databases. This commit adjusts Skeema's Docker-related
logic to explicitly disable the binlog when creating new ephemeral DB
containers, both in workspace=docker and in internal test suites.

Disabling the binlog in these ephemeral containers can improve write perf. It
also simplifies some code paths and test .sql files which previously
explicitly set sql_log_bin=0 at the session level.

This change is also advantageous for MySQL 8 workspace=docker users who use
CREATE FUNCTION statements that lack the DETERMINISTIC, NO SQL, or READS SQL
DATA characteristics. Previously, such statements would result in MySQL error
1418, even if Skeema's user has SUPER privs and sql_log_bin=0 is used at the
session level.

Although SUPER prevents the similar error 1419, it does not overcome error
1418. The only solutions are to disable binlogging entirely (which requires a
server restart); or add one of the necessary characteristics to the function;
or enable log_bin_trust_function_creators=1 (which is global-only, no session
equivalent so not controllable via Skeema's connect-options). By disabling
binlogging for new MySQL 8 containers automatically, this problem is solved
cleanly without user intervention.

Existing MySQL 8 containers are unaffected by this change, even if those
containers were previously created by Skeema (either workspace=docker without
docker-cleanup=destroy, or previous runs of Skeema's integration test suite).
Users can manually rm those containers and have Skeema create new ones if
desired.
  • Loading branch information
evanelias committed Nov 29, 2022
1 parent 412a326 commit 2804a44
Show file tree
Hide file tree
Showing 26 changed files with 41 additions and 30 deletions.
1 change: 1 addition & 0 deletions internal/applier/applier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func (s *ApplierIntegrationSuite) Setup(backend string) error {
Name: containerName,
Image: backend,
RootPassword: "fakepw",
CommandArgs: []string{"--skip-log-bin"}, // override MySQL 8 default of enabling binlog
})
return err
})
Expand Down
1 change: 1 addition & 0 deletions internal/dumper/dumper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ func (s *IntegrationSuite) Setup(backend string) (err error) {
Name: fmt.Sprintf("skeema-test-%s", tengo.ContainerNameForImage(backend)),
Image: backend,
RootPassword: "fakepw",
CommandArgs: []string{"--skip-log-bin"}, // override MySQL 8 default of enabling binlog
}
s.d, err = s.manager.GetOrCreateInstance(opts)
return err
Expand Down
1 change: 1 addition & 0 deletions internal/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ func (s *IntegrationSuite) Setup(backend string) (err error) {
Image: backend,
RootPassword: "fakepw",
DefaultConnParams: "foreign_key_checks=0&sql_mode=%27NO_ENGINE_SUBSTITUTION%27", // disabling strict mode to allow zero dates in testdata
CommandArgs: []string{"--skip-log-bin"}, // override MySQL 8 default of enabling binlog
})
if err != nil {
return err
Expand Down
8 changes: 4 additions & 4 deletions internal/tengo/tengo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ type TengoIntegrationSuite struct {

func (s *TengoIntegrationSuite) Setup(backend string) (err error) {
opts := DockerizedInstanceOptions{
Name: fmt.Sprintf("skeema-test-%s", ContainerNameForImage(backend)),
Image: backend,
RootPassword: "fakepw",
DefaultConnParams: "sql_log_bin=0",
Name: fmt.Sprintf("skeema-test-%s", ContainerNameForImage(backend)),
Image: backend,
RootPassword: "fakepw",
CommandArgs: []string{"--skip-log-bin"}, // override MySQL 8 default of enabling binlog
}
s.d, err = s.manager.GetOrCreateInstance(opts)
return err
Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/check-maria.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Tables using MariaDB's check constraints

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/check.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
# some syntactical differences.

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/colcompression-maria.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Table using MariaDB's column compression

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/colcompression-percona.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Table using Percona Server's column compression

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/default-expr-maria.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@


SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/default-expr.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# char support there.)

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/ft-parser.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# preinstalled in MySQL 5.7+)

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/generatedcols-maria.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
# generated columns, so it is excluded from related tests.

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/generatedcols.sql
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
# support in generation expressions.

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/index-maria106.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Coverage for ignored (invisible) indexes in MariaDB 10.6

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/index-mysql8.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Advanced index features present in MySQL 8+

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/integration-ext.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# a majority of tests, so they're separated out from integration.sql.

SET foreign_key_checks=0;
SET sql_log_bin=0;
CREATE DATABASE testcollate DEFAULT COLLATE latin1_bin;
CREATE DATABASE testcharset DEFAULT CHARACTER SET utf8mb4;
CREATE DATABASE testcharcoll DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
Expand Down
3 changes: 1 addition & 2 deletions internal/tengo/testdata/integration.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
SET foreign_key_checks=0;
SET sql_log_bin=0;
CREATE DATABASE testing;

use testing
Expand Down Expand Up @@ -96,6 +95,6 @@ returns float deterministic NO SQL COMMENT 'hello world' return mult * 2.0;
CREATE FUNCTION func2( num int,
name varchar(30) character set utf8mb4
)
returns varchar(30) character set utf8mb4 deterministic
returns varchar(30) character set utf8mb4
return REPEAT(CONCAT('it''s 💩', name, '! '), num);

1 change: 0 additions & 1 deletion internal/tengo/testdata/inviscols.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
# due to a MariaDB bug; see MDEV-20210

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/maria108.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# with IN / OUT / INOUT param qualifiers

SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/partition.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
SET foreign_key_checks=0;
SET sql_log_bin=0;
CREATE DATABASE partitionparty DEFAULT CHARSET=latin1;
use partitionparty

Expand Down
1 change: 0 additions & 1 deletion internal/tengo/testdata/rows.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
SET foreign_key_checks=0;
SET sql_log_bin=0;

use testing

Expand Down
3 changes: 2 additions & 1 deletion internal/workspace/localdocker.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ func NewLocalDocker(opts Options) (_ *LocalDocker, retErr error) {
if cstore.containers[opts.ContainerName] != nil {
ld.d = cstore.containers[opts.ContainerName]
} else {
var commandArgs []string
commandArgs := []string{"--skip-log-bin"} // override MySQL 8 default of enabling binlog (never needed in workspace)

// If real inst had lower_case_table_names=1, use that in the container as
// well. (No need for similar logic with lower_case_table_names=2; this cannot
// be used on Linux, and code in ExecLogicalSchema already gets us close
Expand Down
1 change: 1 addition & 0 deletions internal/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ func (s *WorkspaceIntegrationSuite) Setup(backend string) (err error) {
Name: fmt.Sprintf("skeema-test-%s", tengo.ContainerNameForImage(backend)),
Image: backend,
RootPassword: "fakepw",
CommandArgs: []string{"--skip-log-bin"}, // override MySQL 8 default of enabling binlog
})
return err
}
Expand Down
4 changes: 2 additions & 2 deletions lctn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (s SkeemaIntegrationSuite) TestLowerCaseTableNames1(t *testing.T) {
// Create an instance with lctn=1
opts := s.d.DockerizedInstanceOptions
opts.Name = strings.Replace(opts.Name, "skeema-test-", "skeema-test-lctn1-", 1)
opts.CommandArgs = []string{"--lower-case-table-names=1"}
opts.CommandArgs = []string{"--skip-log-bin", "--lower-case-table-names=1"}
dinst, err := s.manager.GetOrCreateInstance(opts)
if err != nil {
t.Fatalf("Unable to create Dockerized instance with lower-case-table-names=1: %v", err)
Expand Down Expand Up @@ -166,7 +166,7 @@ func (s SkeemaIntegrationSuite) TestLowerCaseTableNames2(t *testing.T) {
// Create an instance with lctn=2
opts := s.d.DockerizedInstanceOptions
opts.Name = strings.Replace(opts.Name, "skeema-test-", "skeema-test-lctn2-", 1)
opts.CommandArgs = []string{"--lower-case-table-names=2"}
opts.CommandArgs = []string{"--skip-log-bin", "--lower-case-table-names=2"}
opts.DataBindMount = t.TempDir()
dinst, err := s.manager.GetOrCreateInstance(opts)
if err != nil {
Expand Down
32 changes: 27 additions & 5 deletions skeema_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1369,14 +1369,37 @@ END`
}
}

// TestTempSchemaBinlog provides coverage for the temp-schema-binlog option.
// Because we ordinarily create containerized test DBs with binlogging disabled
// (even in MySQL 8 where it normally defaults to enabled), this test has to
// create a new separate container for its logic.
// This test is run in CI, or when SKEEMA_TEST_BINLOG env var is set to any non-
// blank value.
func (s SkeemaIntegrationSuite) TestTempSchemaBinlog(t *testing.T) {
if !s.d.Flavor().Min(tengo.FlavorMySQL80) {
t.Skip("Test only relevant for flavors that default to having binlog enabled")
if os.Getenv("SKEEMA_TEST_BINLOG") == "" && (os.Getenv("CI") == "" || os.Getenv("CI") == "0" || os.Getenv("CI") == "false") {
t.Skip("Skipping temp-schema-binlog testing. To run, set env var SKEEMA_TEST_BINLOG=true and/or CI=1.")
}

// Create an instance with log-bin enabled
opts := s.d.DockerizedInstanceOptions
opts.Name = strings.Replace(opts.Name, "skeema-test-", "skeema-test-binlog-", 1)
opts.CommandArgs = []string{"--log-bin", "--server-id=1"}
dinst, err := s.manager.GetOrCreateInstance(opts)
if err != nil {
t.Fatalf("Unable to create Dockerized instance with log-bin enabled: %v", err)
}
defer func() {
if err := dinst.Destroy(); err != nil {
t.Errorf("Unable to destroy test instance with log-bin enabled: %v", err)
}
}()
if _, err := dinst.SourceSQL("../setup.sql"); err != nil {
t.Fatalf("Unable to source setup.sql: %v", err)
}

getLogPos := func() string {
t.Helper()
db, err := s.d.CachedConnectionPool("", "")
db, err := dinst.CachedConnectionPool("", "")
if err != nil {
t.Fatalf("Unable to establish connection: %v", err)
}
Expand Down Expand Up @@ -1410,9 +1433,8 @@ func (s SkeemaIntegrationSuite) TestTempSchemaBinlog(t *testing.T) {
}

pos := getLogPos()
s.handleCommand(t, CodeSuccess, ".", "skeema init --dir mydb -h %s -P %d", s.d.Instance.Host, s.d.Instance.Port)
s.handleCommand(t, CodeSuccess, ".", "skeema init --dir mydb -h %s -P %d", dinst.Instance.Host, dinst.Instance.Port)
assertNotLogged(pos)
s.dbExec(t, "analytics", "ALTER TABLE pageviews DROP COLUMN domain")
createRoutine := `CREATE definer=root@localhost FUNCTION routine1(a int,
b int)
RETURNS int
Expand Down
1 change: 1 addition & 0 deletions skeema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func (s *SkeemaIntegrationSuite) Setup(backend string) (err error) {
Name: fmt.Sprintf("skeema-test-%s", tengo.ContainerNameForImage(backend)),
Image: backend,
RootPassword: "fakepw",
CommandArgs: []string{"--skip-log-bin"}, // override MySQL 8 default of enabling binlog
}
s.d, err = s.manager.GetOrCreateInstance(opts)
return err
Expand Down

0 comments on commit 2804a44

Please sign in to comment.