Comment 8 for bug 1704072

Revision history for this message
John A Meinel (jameinel) wrote :

The function that assigns replicaset ids is here:
https://github.com/juju/replicaset/blob/master/replicaset.go#L290

From what I read of that code, it takes a lists of members to set and compares it with the current config to figure out what Ids might already be in use.

It starts with 'max == 0', and then goes through the existing config.Members to find the ids, and compares it with max.
It then goes through the members, and for members that *don't* have an Id already set, it sets them to max++.

So one *possibility*, would be that we somehow ended up with a CurrentConfig that has a member with replicaset Id 0 (thus not originally changing 'max') and then the members list we are passing in *also* has a member with Id = 1, and other members without their Ids set.

I think a potential fix here is that we should also iterate over the 'members' list and consider those ids also as part of the potential values that we need to avoid.

I haven't found any way (yet) to actually trigger this configuration, its only been by code inspection that I've found any way to match the comment 30 from bug #1701275.

Something like this:
diff --git a/replicaset.go b/replicaset.go
index ed9cc48..7440435 100644
--- a/replicaset.go
+++ b/replicaset.go
@@ -307,6 +307,12 @@ func Set(session *mgo.Session, members []Member) error {
                        max = m.Id
                }
        }
+ // Also check if any of the members being passed in already have an Id that we would be reusing.
+ for _, m := range members {
+ if m.Id > max {
+ max = m.Id
+ }
+ }

        for x, m := range members {
                if id, ok := ids[m.Address]; ok {