Skip to content
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

Use ExecutedGtidSet and RetrievedGtidSet when determining the primary #474

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

d-kuro
Copy link
Collaborator

@d-kuro d-kuro commented Nov 16, 2022

refs: #370

Use Executed_Gtid_Set and Retrieved_Gtid_Set when determining the primary.

Previously, only Retrieved_Gtid_Set was used to select the primary.
However, there are cases where Retrieved_Gtid_Set is empty immediately after replication starts if there are no new transactions.

$ kubectl-moco mysql test --index 2 -- -e "SHOW REPLICA STATUS\G"
*************************** 1. row ***************************
             Replica_IO_State: Waiting for source to send event
...
           Retrieved_Gtid_Set:
            Executed_Gtid_Set: 09a2bf6a-960c-11ec-bf89-9ee9543e8238:1,
09c0848a-960c-11ec-90e9-0a55db88221e:1-4
...

Therefore, change to also use ExecutedGtidSet for finding primary.

// There are cases where Retrieved_Gtid_Set is empty,
// such as when there is no transaction immediately after a fail-over.
// Therefore, Retrieved_Gtid_Set and Executed_Gtid_Set are unioned to find for the top runner.
// The union of two GTID sets is simply their joined together with an interposed comma.
// https://dev.mysql.com/doc/refman/8.0/en/gtid-functions.html
var gtids string
if len(repl.RetrievedGtidSet) == 0 {
	gtids = repl.ExecutedGtidSet
} else {
	gtids = fmt.Sprintf("%s,%s", repl.RetrievedGtidSet, repl.ExecutedGtidSet)
}

Steps

  1. Submitting data
$ kubectl-moco mysql test -it -u moco-writable -- -e "CREATE DATABASE test;"
$ kubectl-moco mysql test -it -u moco-writable -- -e "CREATE TABLE test.t1 (foo int);"
$ kubectl-moco mysql test -it -u moco-writable -- -e "INSERT INTO test.t1 (foo) VALUES (1); COMMIT;"
  1. Kills the primary and causes failover
$ kubectl get mysqlcluster test
$ PRIMARY=$(kubectl get mysqlcluster test -o jsonpath='{.status.currentPrimaryIndex}')
$ kubectl exec -it moco-test-${PRIMARY} -c mysqld -- kill 1
  1. Kill the primary again
$ PRIMARY=$(kubectl get mysqlcluster test -o jsonpath='{.status.currentPrimaryIndex}')
$ kubectl exec -it moco-test-${PRIMARY} -c mysqld -- kill 1
  1. The following log is output and failover fails
{"level":"error","ts":1646976101.8021455,"logger":"cluster-manager.default/test","msg":"error","error":"failed to failover: failed to choose the next primary: unable to determine the top runner"}

@d-kuro d-kuro self-assigned this Nov 16, 2022
@d-kuro d-kuro force-pushed the d-kuro/fix-failover branch 4 times, most recently from 43800f8 to e9fbdc7 Compare December 21, 2022 01:16
…the primary

Signed-off-by: d-kuro <kurosawa7620@gmail.com>
@d-kuro d-kuro changed the title Use ExecutedGtidSet if there is no RetrievedGtidSet when determining the primary Use ExecutedGtidSet and RetrievedGtidSet when determining the primary Jan 18, 2023
@masa213f masa213f self-requested a review January 20, 2023 07:35
e2e/failover_test.go Show resolved Hide resolved
Signed-off-by: d-kuro <kurosawa7620@gmail.com>
@d-kuro d-kuro marked this pull request as ready for review January 24, 2023 19:43
@masa213f masa213f self-requested a review January 25, 2023 02:54
if err != nil {
return err
}
if currentPrimaryIndex == cluster.Status.CurrentPrimaryIndex {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of currentPrimaryIndex is out of date here.
Could you update currentPrimaryIndex after line 104?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review. I fixed the issue as it was not correctly evaluating. Thank you again.

c823871

Signed-off-by: d-kuro <kurosawa7620@gmail.com>
@d-kuro d-kuro requested a review from masa213f January 31, 2023 00:15
Copy link
Contributor

@masa213f masa213f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@masa213f masa213f merged commit c020d45 into main Feb 1, 2023
@masa213f masa213f deleted the d-kuro/fix-failover branch February 1, 2023 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants