-
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?
Conversation
canal/canal.go
Outdated
@@ -11,13 +11,13 @@ import ( | |||
"sync" | |||
"time" | |||
|
|||
"github.com/bytewatch/dolphinbeat/schema" |
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.
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.
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.
@@ -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 comment
The 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 comment
The 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.
还没有合并进去么,非常期待这个优化 @siddontang @bytewatch |
@bytewatch hi, you haven’t updated this pr for a long time, and I just want to know what is your plan now? |
@domyway I think I can push it. |
@3pointer now please take charge of this pr |
@bytewatch is this still something you are interested in? If so can we resolve the conflicts and/or rebase on the latest master? |
Backporting schema feature from dolphinbeat.
Feature:
Warning:
TODO: