From df7f7bbd29d64bb6620e13000482f8bd3c96334f Mon Sep 17 00:00:00 2001 From: shlomi-noach Date: Thu, 12 Nov 2015 13:50:57 +0100 Subject: [PATCH] MakeCoMaster: We allow breaking of an existing co-master replication. Here's the breakdown: Ideally, this would not eb allowed, and we would first require the user to RESET SLAVE on 'master' prior to making it participate as co-master with our 'instance'. However there's the problem that upon RESET SLAVE we lose the replication's user/password info. Thus, we come up with the following rule: If S replicates from M1, and M1<->M2 are co masters, we allow S to become co-master of M1 (S<->M1) if: - M1 is writeable - M2 is read-only or is unreachable/invalid - S is read-only And so we will be replacing one read-only co-master with another. (instance_topology.go) supported by Web interface (cluster.js) --- build.sh | 2 +- go/inst/instance.go | 6 ----- go/inst/instance_topology.go | 44 ++++++++++++++++++++++++---------- resources/public/js/cluster.js | 18 +++++++++++++- 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/build.sh b/build.sh index 81fbe75a..e59ef586 100755 --- a/build.sh +++ b/build.sh @@ -6,7 +6,7 @@ # set -e -RELEASE_VERSION="1.4.513" +RELEASE_VERSION="1.4.515" TOPDIR=/tmp/orchestrator-release export RELEASE_VERSION TOPDIR diff --git a/go/inst/instance.go b/go/inst/instance.go index e4de364b..b9af2b9b 100644 --- a/go/inst/instance.go +++ b/go/inst/instance.go @@ -325,12 +325,6 @@ func (this *Instance) CanMoveAsCoMaster() (bool, error) { if !this.IsRecentlyChecked { return false, fmt.Errorf("%+v: not recently checked", this.Key) } - if this.Slave_SQL_Running { - return false, fmt.Errorf("%+v: instance is replicating", this.Key) - } - if this.Slave_IO_Running { - return false, fmt.Errorf("%+v: instance is replicating", this.Key) - } return true, nil } diff --git a/go/inst/instance_topology.go b/go/inst/instance_topology.go index 09c3900d..1f11b11d 100644 --- a/go/inst/instance_topology.go +++ b/go/inst/instance_topology.go @@ -817,25 +817,41 @@ func MakeCoMaster(instanceKey *InstanceKey) (*Instance, error) { if err != nil { return instance, err } + if canMove, merr := instance.CanMove(); !canMove { + return instance, merr + } master, err := GetInstanceMaster(instance) if err != nil { return instance, err } - - rinstance, _, _ := ReadInstance(&master.Key) - if canMove, merr := rinstance.CanMoveAsCoMaster(); !canMove { - return instance, merr - } - rinstance, _, _ = ReadInstance(instanceKey) - if canMove, merr := rinstance.CanMove(); !canMove { + log.Debugf("Will check whether %+v's master (%+v) can become its co-master", instance.Key, master.Key) + if canMove, merr := master.CanMoveAsCoMaster(); !canMove { return instance, merr } - if instanceKey.Equals(&master.MasterKey) { - return instance, fmt.Errorf("instance %+v is already co master of %+v", instanceKey, master.Key) - } - if _, found, _ := ReadInstance(&master.MasterKey); found { - return instance, fmt.Errorf("master %+v already has known master: %+v", master.Key, master.MasterKey) + return instance, fmt.Errorf("instance %+v is already co master of %+v", instance.Key, master.Key) + } + if !instance.ReadOnly { + return instance, fmt.Errorf("instance %+v is not read-only; first make it read-only before making it co-master", instance.Key) + } + if master.IsCoMaster { + // We allow breaking of an existing co-master replication. Here's the breakdown: + // Ideally, this would not eb allowed, and we would first require the user to RESET SLAVE on 'master' + // prior to making it participate as co-master with our 'instance'. + // However there's the problem that upon RESET SLAVE we lose the replication's user/password info. + // Thus, we come up with the following rule: + // If S replicates from M1, and M1<->M2 are co masters, we allow S to become co-master of M1 (S<->M1) if: + // - M1 is writeable + // - M2 is read-only or is unreachable/invalid + // - S is read-only + // And so we will be replacing one read-only co-master with another. + otherCoMaster, found, _ := ReadInstance(&master.MasterKey) + if found && otherCoMaster.IsLastCheckValid && !otherCoMaster.ReadOnly { + return instance, fmt.Errorf("master %+v is already co-master with %+v, and %+v is alive, and not read-only; cowardly refusing to demote it. Please set it as read-only beforehand", master.Key, otherCoMaster.Key, otherCoMaster.Key) + } + // OK, good to go. + } else if _, found, _ := ReadInstance(&master.MasterKey); found { + return instance, fmt.Errorf("%+v is not a real master; it replicates from: %+v", master.Key, master.MasterKey) } if canReplicate, err := master.CanReplicateFrom(instance); !canReplicate { return instance, err @@ -857,6 +873,10 @@ func MakeCoMaster(instanceKey *InstanceKey) (*Instance, error) { // the coMaster used to be merely a slave. Just point master into *some* position // within coMaster... + master, err = StopSlave(&master.Key) + if err != nil { + goto Cleanup + } master, err = ChangeMasterTo(&master.Key, instanceKey, &instance.SelfBinlogCoordinates, false, GTIDHintNeutral) if err != nil { goto Cleanup diff --git a/resources/public/js/cluster.js b/resources/public/js/cluster.js index e9046f54..9ed0e1a9 100644 --- a/resources/public/js/cluster.js +++ b/resources/public/js/cluster.js @@ -407,8 +407,24 @@ function Cluster() { if (node.id == droppableNode.id) { return { accept: false }; } + if (instanceIsChild(droppableNode, node) && node.isCoMaster) { + // We may allow a co-master to change its othe rco-master under some conditions, + // see MakeCoMaster() in instance_topology.go + if (!droppableNode.ReadOnly) { + return { accept: false }; + } + var coMaster = node.masterNode; + if (coMaster.id == droppableNode.id) { + return { accept: false }; + } + if (coMaster.lastCheckInvalidProblem() || coMaster.notRecentlyCheckedProblem() || coMaster.ReadOnly) { + if (shouldApply) { + makeCoMaster(node, droppableNode); + } + return { accept: "ok", type: "makeCoMaster with " + droppableTitle }; + } + } if (node.isCoMaster) { - // Cannot move. RESET SLAVE on one of the co-masters. return { accept: false }; } if (instancesAreSiblings(node, droppableNode)) {