-
Notifications
You must be signed in to change notification settings - Fork 987
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.
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
backport schema feature from dolphinbeat #358
base: master
Are you sure you want to change the base?
Changes from 13 commits
2c5a50b
4b772da
152ef50
72c589f
59d83a6
503c6b4
3fb4533
12d3dda
0354762
29286f0
3da28fe
7e87fa6
5dd1cf0
c29ef1a
7b4e757
5f8859c
3ec35d1
c1ba9b3
80b9095
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,9 @@ import ( | |
) | ||
|
||
var testHost = flag.String("host", "127.0.0.1", "MySQL host") | ||
var testPort = flag.Int("port", 3306, "MySQL port") | ||
var testUser = flag.String("user", "root", "MySQL user") | ||
var testPassword = flag.String("password", "", "MySQL password") | ||
|
||
func Test(t *testing.T) { | ||
TestingT(t) | ||
|
@@ -27,8 +30,9 @@ var _ = Suite(&canalTestSuite{}) | |
|
||
func (s *canalTestSuite) SetUpSuite(c *C) { | ||
cfg := NewDefaultConfig() | ||
cfg.Addr = fmt.Sprintf("%s:3306", *testHost) | ||
cfg.User = "root" | ||
cfg.Addr = fmt.Sprintf("%s:%d", *testHost, *testPort) | ||
cfg.User = *testUser | ||
cfg.Password = *testPassword | ||
cfg.HeartbeatPeriod = 200 * time.Millisecond | ||
cfg.ReadTimeout = 300 * time.Millisecond | ||
cfg.Dump.ExecutionPath = "mysqldump" | ||
|
@@ -61,8 +65,6 @@ func (s *canalTestSuite) SetUpSuite(c *C) { | |
s.execute(c, "DELETE FROM test.canal_test") | ||
s.execute(c, "INSERT INTO test.canal_test (content, name) VALUES (?, ?), (?, ?), (?, ?)", "1", "a", `\0\ndsfasdf`, "b", "", "c") | ||
|
||
s.execute(c, "SET GLOBAL binlog_format = 'ROW'") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why remove this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because my account dosen't have super privilege to set this global variable. And I think it's ok if mysql is configured to use ROW format already, like the travis config. |
||
|
||
s.c.SetEventHandler(&testEventHandler{c: c}) | ||
go func() { | ||
err = s.c.Run() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Em, can you put all to go-mysql and not import any dolphinbeat? It is very strange for circular dependence. E.g, go-mysql depends dolphinebeat, and dolphinebeat depends on go-mysql.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have copied the source from dolpinbeat/schema to go-mysql/schema. Now I have a build problem under go 1.9 becuase pingcap/parser depends on
strings.Builder
: Build 655. Do you have any idea?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bytewatch
I think we don't need to consider go 1.9 now, we will support at 1.11 at least later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will support Go mod and remove vendor at first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok to build now, PTAL. By the way, can i ask a question about TiDB's Percolator implemention? I wonder can it turn to 1PC if all mutations is in one region to improve perfermance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bytewatch
There is a corner-case for 1 PC, and if we want to do this, we still need to do some extra works, so this has a lower priority for us. If you want to have a try, you can discuss with us in TiKV issues directly.