Only log changes in read replica topology, not the whole thing

enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/discovery/CoreTopology.java

@@ -69,11 +69,10 @@ public Optional<CoreServerInfo> find( MemberId memberId )
     @Override
     public String toString()
     {
-        return format( "{clusterId=%s, bootstrappable=%s, coreMembers=%s}", clusterId, canBeBootstrapped(),
-                coreMembers );
+        return format( "{clusterId=%s, bootstrappable=%s, coreMembers=%s}", clusterId, canBeBootstrapped(), coreMembers );
     }
 
-    TopologyDifference difference( CoreTopology other )
+    CoreTopologyDifference difference( CoreTopology other )
     {
         Set<MemberId> members = coreMembers.keySet();
         Set<MemberId> otherMembers = other.coreMembers.keySet();
@@ -84,7 +83,7 @@ TopologyDifference difference( CoreTopology other )
         Set<Difference> removed = members.stream().filter( m -> !otherMembers.contains( m ) )
                 .map( memberId -> asDifference( CoreTopology.this, memberId ) ).collect( toSet() );
 
-        return new TopologyDifference( added, removed );
+        return new CoreTopologyDifference( added, removed );
     }
 
     private Difference asDifference( CoreTopology topology, MemberId memberId )
@@ -97,12 +96,12 @@ public Optional<MemberId> anyCoreMemberId()
             return coreMembers.keySet().stream().findAny();
     }
 
-    class TopologyDifference
+    class CoreTopologyDifference
     {
         private Set<Difference> added;
         private Set<Difference> removed;
 
-        TopologyDifference( Set<Difference> added, Set<Difference> removed )
+        CoreTopologyDifference( Set<Difference> added, Set<Difference> removed )
         {
             this.added = added;
             this.removed = removed;

enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/discovery/HazelcastCoreTopologyService.java

@@ -253,7 +253,7 @@ private void refreshCoreTopology()
     {
         CoreTopology newCoreTopology = getCoreTopology( hazelcastInstance, config, log );
 
-        CoreTopology.TopologyDifference difference = coreTopology.difference( newCoreTopology );
+        CoreTopology.CoreTopologyDifference difference = coreTopology.difference( newCoreTopology );
         if ( difference.hasChanges() )
         {
             log.info( "Core topology changed %s", difference );
@@ -265,8 +265,15 @@ private void refreshCoreTopology()
 
     private void refreshReadReplicaTopology()
     {
-        readReplicaTopology = getReadReplicaTopology( hazelcastInstance, log );
-        log.info( "Current read replica topology is %s", readReplicaTopology );
+        ReadReplicaTopology newReadReplicaTopology = getReadReplicaTopology( hazelcastInstance, log );
+
+        ReadReplicaTopology.ReadReplicaTopologyDifference difference = readReplicaTopology.difference( newReadReplicaTopology );
+        if ( difference.hasChanges() )
+        {
+            log.info( "Read replica topology changed %s", difference );
+        }
+
+        this.readReplicaTopology = newReadReplicaTopology;
     }
 
     private class OurMembershipListener implements MembershipListener

enterprise/causal-clustering/src/main/java/org/neo4j/causalclustering/discovery/ReadReplicaTopology.java

@@ -22,10 +22,12 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 import org.neo4j.causalclustering.identity.MemberId;
 
 import static java.util.Collections.emptyMap;
+import static java.util.stream.Collectors.toSet;
 
 
 public class ReadReplicaTopology
@@ -70,6 +72,75 @@ public Optional<MemberId> anyReadReplicaMemberId()
         {
             return readReplicaMembers.keySet().stream().findAny();
         }
+    }
+
+    ReadReplicaTopologyDifference difference( ReadReplicaTopology other )
+    {
+        Set<MemberId> members = readReplicaMembers.keySet();
+        Set<MemberId> otherMembers = other.readReplicaMembers.keySet();
+
+        Set<ReadReplicaTopology.Difference> added = otherMembers.stream().filter( m -> !members.contains( m ) )
+                .map( memberId -> asDifference( other, memberId ) ).collect( toSet() );
+
+        Set<ReadReplicaTopology.Difference> removed = members.stream().filter( m -> !otherMembers.contains( m ) )
+                .map( memberId -> asDifference( ReadReplicaTopology.this, memberId ) ).collect( toSet() );
+
+        return new ReadReplicaTopologyDifference( added, removed );
+    }
+
+    private ReadReplicaTopology.Difference asDifference( ReadReplicaTopology topology, MemberId memberId )
+    {
+        return new ReadReplicaTopology.Difference( memberId, topology.find( memberId ).orElse( null ) );
+    }
+
+    class ReadReplicaTopologyDifference
+    {
+        private Set<ReadReplicaTopology.Difference> added;
+        private Set<ReadReplicaTopology.Difference> removed;
+
+        ReadReplicaTopologyDifference( Set<ReadReplicaTopology.Difference> added, Set<ReadReplicaTopology.Difference> removed )
+        {
+            this.added = added;
+            this.removed = removed;
+        }
+
+        Set<ReadReplicaTopology.Difference> added()
+        {
+            return added;
+        }
 
+        Set<ReadReplicaTopology.Difference> removed()
+        {
+            return removed;
+        }
+
+        boolean hasChanges()
+        {
+            return added.size() > 0 || removed.size() > 0;
+        }
+
+        @Override
+        public String toString()
+        {
+            return String.format( "{added=%s, removed=%s}", added, removed );
+        }
+    }
+
+    private class Difference
+    {
+        private MemberId memberId;
+        private ReadReplicaInfo readReplicaInfo;
+
+        Difference( MemberId memberId, ReadReplicaInfo readReplicaInfo )
+        {
+            this.memberId = memberId;
+            this.readReplicaInfo = readReplicaInfo;
+        }
+
+        @Override
+        public String toString()
+        {
+            return String.format( "{memberId=%s, readReplicaInfo=%s}", memberId, readReplicaInfo );
+        }
     }
 }

enterprise/causal-clustering/src/test/java/org/neo4j/causalclustering/discovery/CoreTopologyTest.java

@@ -48,7 +48,7 @@ public void identicalTopologiesShouldHaveNoDifference() throws Exception
         CoreTopology topology = new CoreTopology( new ClusterId( UUID.randomUUID() ), true, coreMembers );
 
         // when
-        CoreTopology.TopologyDifference diff =  topology.difference(topology);
+        CoreTopology.CoreTopologyDifference diff =  topology.difference(topology);
 
         // then
         assertThat( diff.added().size(), Matchers.equalTo( 0 ) );
@@ -74,7 +74,7 @@ public void shouldDetectAddedMembers() throws Exception
         CoreTopology topology = new CoreTopology( new ClusterId( UUID.randomUUID() ), true, initialMembers );
 
         // when
-        CoreTopology.TopologyDifference diff =  topology.difference(new CoreTopology( new ClusterId( UUID.randomUUID() ), true, newMembers ));
+        CoreTopology.CoreTopologyDifference diff =  topology.difference(new CoreTopology( new ClusterId( UUID.randomUUID() ), true, newMembers ));
 
         // then
         assertThat( diff.added().size(), Matchers.equalTo( 1 ) );
@@ -98,7 +98,7 @@ public void shouldDetectRemovedMembers() throws Exception
         CoreTopology topology = new CoreTopology( new ClusterId( UUID.randomUUID() ), true, initialMembers );
 
         // when
-        CoreTopology.TopologyDifference diff =  topology.difference(new CoreTopology( new ClusterId( UUID.randomUUID() ), true, newMembers ));
+        CoreTopology.CoreTopologyDifference diff =  topology.difference(new CoreTopology( new ClusterId( UUID.randomUUID() ), true, newMembers ));
 
         // then
         assertThat( diff.added().size(), Matchers.equalTo( 0 ) );
@@ -121,7 +121,7 @@ public void shouldDetectAddedAndRemovedMembers() throws Exception
         CoreTopology topology = new CoreTopology( new ClusterId( UUID.randomUUID() ), true, initialMembers );
 
         // when
-        CoreTopology.TopologyDifference diff =  topology.difference(new CoreTopology( new ClusterId( UUID.randomUUID() ), true, newMembers ));
+        CoreTopology.CoreTopologyDifference diff =  topology.difference(new CoreTopology( new ClusterId( UUID.randomUUID() ), true, newMembers ));
 
         // then
         assertThat( diff.added().size(), Matchers.equalTo( 2 ) );

enterprise/causal-clustering/src/test/java/org/neo4j/causalclustering/discovery/ReadReplicaTopologyTest.java

@@ -0,0 +1,130 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.causalclustering.discovery;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.hamcrest.Matchers;
+import org.junit.Test;
+
+import org.neo4j.causalclustering.identity.ClusterId;
+import org.neo4j.causalclustering.identity.MemberId;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.mock;
+
+public class ReadReplicaTopologyTest
+{
+    @Test
+    public void identicalTopologiesShouldHaveNoDifference() throws Exception
+    {
+        // given
+        UUID one = UUID.randomUUID();
+        UUID two = UUID.randomUUID();
+
+        Map<MemberId,ReadReplicaInfo> readReplicaMembers = new HashMap<>();
+        readReplicaMembers.put( new MemberId( one ), mock(ReadReplicaInfo.class) );
+        readReplicaMembers.put( new MemberId( two ), mock(ReadReplicaInfo.class) );
+
+        ReadReplicaTopology topology = new ReadReplicaTopology( readReplicaMembers );
+
+        // when
+        ReadReplicaTopology.ReadReplicaTopologyDifference diff =  topology.difference(topology);
+
+        // then
+        assertThat( diff.added().size(), Matchers.equalTo( 0 ) );
+        assertThat( diff.removed().size(), Matchers.equalTo( 0 ) );
+    }
+
+    @Test
+    public void shouldDetectAddedMembers() throws Exception
+    {
+        // given
+        UUID one = UUID.randomUUID();
+        UUID two = UUID.randomUUID();
+
+        Map<MemberId,ReadReplicaInfo> initialMembers = new HashMap<>();
+        initialMembers.put( new MemberId( one ), mock(ReadReplicaInfo.class) );
+        initialMembers.put( new MemberId( two ), mock(ReadReplicaInfo.class) );
+
+        Map<MemberId,ReadReplicaInfo> newMembers = new HashMap<>();
+        newMembers.put( new MemberId( one ), mock(ReadReplicaInfo.class) );
+        newMembers.put( new MemberId( two ), mock(ReadReplicaInfo.class) );
+        newMembers.put( new MemberId( UUID.randomUUID() ), mock(ReadReplicaInfo.class) );
+
+        ReadReplicaTopology topology = new ReadReplicaTopology( initialMembers );
+
+        // when
+        ReadReplicaTopology.ReadReplicaTopologyDifference diff =  topology.difference(new ReadReplicaTopology( newMembers ));
+
+        // then
+        assertThat( diff.added().size(), Matchers.equalTo( 1 ) );
+        assertThat( diff.removed().size(), Matchers.equalTo( 0 ) );
+    }
+
+    @Test
+    public void shouldDetectRemovedMembers() throws Exception
+    {
+        // given
+        UUID one = UUID.randomUUID();
+        UUID two = UUID.randomUUID();
+
+        Map<MemberId,ReadReplicaInfo> initialMembers = new HashMap<>();
+        initialMembers.put( new MemberId( one ), mock(ReadReplicaInfo.class) );
+        initialMembers.put( new MemberId( two ), mock(ReadReplicaInfo.class) );
+
+        Map<MemberId,ReadReplicaInfo> newMembers = new HashMap<>();
+        newMembers.put( new MemberId( two ), mock(ReadReplicaInfo.class) );
+
+        ReadReplicaTopology topology = new ReadReplicaTopology( initialMembers );
+
+        // when
+        ReadReplicaTopology.ReadReplicaTopologyDifference diff =  topology.difference(new ReadReplicaTopology( newMembers ));
+
+        // then
+        assertThat( diff.added().size(), Matchers.equalTo( 0 ) );
+        assertThat( diff.removed().size(), Matchers.equalTo( 1 ) );
+    }
+
+    @Test
+    public void shouldDetectAddedAndRemovedMembers() throws Exception
+    {
+        // given
+
+        Map<MemberId,ReadReplicaInfo> initialMembers = new HashMap<>();
+        initialMembers.put( new MemberId( UUID.randomUUID() ), mock(ReadReplicaInfo.class) );
+        initialMembers.put( new MemberId( UUID.randomUUID() ), mock(ReadReplicaInfo.class) );
+
+        Map<MemberId,ReadReplicaInfo> newMembers = new HashMap<>();
+        newMembers.put( new MemberId( UUID.randomUUID() ), mock(ReadReplicaInfo.class) );
+        newMembers.put( new MemberId( UUID.randomUUID() ), mock(ReadReplicaInfo.class) );
+
+        ReadReplicaTopology topology = new ReadReplicaTopology( initialMembers );
+
+        // when
+        ReadReplicaTopology.ReadReplicaTopologyDifference diff =  topology.difference(new ReadReplicaTopology( newMembers ));
+
+        // then
+        assertThat( diff.added().size(), Matchers.equalTo( 2 ) );
+        assertThat( diff.removed().size(), Matchers.equalTo( 2 ) );
+    }
+}