-
Notifications
You must be signed in to change notification settings - Fork 86
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
*: Prevent Leader from accidentally hanging up data inconsistency #597
Conversation
c14eb2a
to
abf7e92
Compare
2. Add leaderstop cmd in xenon conf 3. Adjust project structure
abf7e92
to
645e893
Compare
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.
LGTM
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.
LGTM
}, | ||
} | ||
|
||
assert.Equal(t, lifeCycle, xenonCase.Lifecycle) |
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.
use "github.com/onsi/gomega" lib, Expect
|
||
"github.com/bitly/go-simplejson" | ||
_ "github.com/go-sql-driver/mysql" | ||
"github.com/jmoiron/sqlx" |
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.
_ "github.com/go-sql-driver/mysql" is ok, Do not use "github.com/jmoiron/sqlx"
cfg := loadConfiguration() | ||
dsn := mysqlUser + ":" + cfg.password + "@tcp(" + mysqlHost + ")/" | ||
server := MySQLServer{DSN: dsn} | ||
Conn, err := server.GetNewDBConn() |
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.
conn
must close when finished
if err != nil { | ||
log.Error(err) | ||
} | ||
IsLeader := IsLeader(Conn) |
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.
when error, to here is will crash.
} | ||
// Step 1: disable event scheduler | ||
log.Info("Disabling event scheduler") | ||
stmt, err := SetEventScheduler(Conn, false) |
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.
the same question.
if err != nil { | ||
log.Error(err) | ||
} | ||
log.Infof("set readonly to true: %s", stmt) |
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.
the same, when error , it will crash.
if err != nil { | ||
log.Error(err) | ||
} | ||
log.Infof("%d,long running writes: %s", num, stmt) |
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.
the same, when error , it will crash.
leaderSet = gtid.ExecutedGTIDSet | ||
} | ||
} | ||
db, err := server.GetNewDBConn() |
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.
db need close.
if err != nil { | ||
log.Fatal(err) | ||
} | ||
contents, err2 := ioutil.ReadAll(xenonConfFile) |
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.
defer xenonConfFile.Close()
should at here.
return false, fmt.Errorf("db connection failed after %s timeout", timeout) | ||
case <-ticker.C: | ||
_, err := sqlx.Connect("mysql", server.DSN) | ||
if err == nil { |
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.
when success, need close.
@@ -8,4 +8,4 @@ spec: | |||
# hostname if empty, use the leader as hostname | |||
hostName: sample-mysql-0 | |||
clusterName: sample | |||
# nfsServerAddress: "" | |||
# nfsServerAddress: "" |
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.
new lines.
@@ -37,4 +37,4 @@ EXPOSE 8801 | |||
VOLUME ["/var/lib/xenon", "/etc/xenon"] | |||
|
|||
ENTRYPOINT ["xenon"] | |||
CMD ["-c", "/etc/xenon/xenon.json"] | |||
CMD ["-c", "/etc/xenon/xenon.json"] |
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.
new lines.
for { | ||
select { | ||
case <-timeoutExceeded: | ||
return false, fmt.Errorf("db connection failed after %s timeout", timeout) |
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.
timeout is int64, use %s is very dangerous.
select { | ||
case <-timeoutExceeded: | ||
return false, fmt.Errorf("db connection failed after %s timeout", timeout) | ||
case <-ticker.C: |
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.
need times to retry. 1 second is two frequent.
*: Prevent Leader from accidentally hanging up data inconsistency
What type of PR is this?
/enhancement
Which issue(s) this PR fixes?
Fixes #598
What this PR does?
Summary:
Special notes for your reviewer?