diff --git a/akka-cluster/src/main/scala/akka/cluster/Member.scala b/akka-cluster/src/main/scala/akka/cluster/Member.scala index dcc7cce527..c5237fb9f7 100644 --- a/akka-cluster/src/main/scala/akka/cluster/Member.scala +++ b/akka-cluster/src/main/scala/akka/cluster/Member.scala @@ -53,15 +53,25 @@ class Member private[cluster] ( * Is this member older, has been part of cluster longer, than another * member. It is only correct when comparing two existing members in a * cluster. A member that joined after removal of another member may be - * considered older than the removed member. Note that is only makes - * sense to compare with other members inside of one data center (upNumber has - * a higher risk of being reused across data centers). // TODO should we enforce this to compare only within DCs? + * considered older than the removed member. + * + * Note that it only makes sense to compare with other members of + * same data center (upNumber has a higher risk of being reused across data centers). + * To avoid mistakes of comparing members of different data centers this + * method will throw `IllegalArgumentException` if the members belong + * to different data centers. */ - def isOlderThan(other: Member): Boolean = + @throws[IllegalArgumentException]("if members from different data centers") + def isOlderThan(other: Member): Boolean = { + if (dataCenter != other.dataCenter) + throw new IllegalArgumentException( + "Comparing members of different data centers with isOlderThan is not allowed. " + + s"[$this] vs. [$other]") if (upNumber == other.upNumber) Member.addressOrdering.compare(address, other.address) < 0 else upNumber < other.upNumber + } def copy(status: MemberStatus): Member = { val oldStatus = this.status @@ -141,6 +151,11 @@ object Member { /** * Sort members by age, i.e. using [[Member#isOlderThan]]. + * + * Note that it only makes sense to compare with other members of + * same data center. To avoid mistakes of comparing members of different + * data centers it will throw `IllegalArgumentException` if the + * members belong to different data centers. */ val ageOrdering: Ordering[Member] = Ordering.fromLessThan[Member] { (a, b) ⇒ a.isOlderThan(b) diff --git a/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcHeartbeatTakingOverSpec.scala b/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcHeartbeatTakingOverSpec.scala index 5a3c312da4..3f777d5d61 100644 --- a/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcHeartbeatTakingOverSpec.scala +++ b/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcHeartbeatTakingOverSpec.scala @@ -85,10 +85,10 @@ abstract class MultiDcHeartbeatTakingOverSpec extends MultiNodeSpec(MultiDcHeart // end of these will be filled in during the initial phase of the test ----------- def refreshOldestMemberHeartbeatStatuses() = { - expectedAlphaHeartbeaterNodes = takeNOldestMembers(_.dataCenter == "alpha", 2) + expectedAlphaHeartbeaterNodes = takeNOldestMembers(dataCenter = "alpha", 2) expectedAlphaHeartbeaterRoles = membersAsRoles(expectedAlphaHeartbeaterNodes) - expectedBetaHeartbeaterNodes = takeNOldestMembers(_.dataCenter == "beta", 2) + expectedBetaHeartbeaterNodes = takeNOldestMembers(dataCenter = "beta", 2) expectedBetaHeartbeaterRoles = membersAsRoles(expectedBetaHeartbeaterNodes) expectedNoActiveHeartbeatSenderRoles = roles.toSet -- (expectedAlphaHeartbeaterRoles union expectedBetaHeartbeaterRoles) @@ -154,7 +154,7 @@ abstract class MultiDcHeartbeatTakingOverSpec extends MultiNodeSpec(MultiDcHeart enterBarrier("after-alpha-monitoring-node-left") implicit val sender = observer.ref - val expectedAlphaMonitoringNodesAfterLeaving = (takeNOldestMembers(_.dataCenter == "alpha", 3).filterNot(_.status == MemberStatus.Exiting)) + val expectedAlphaMonitoringNodesAfterLeaving = (takeNOldestMembers(dataCenter = "alpha", 3).filterNot(_.status == MemberStatus.Exiting)) runOn(membersAsRoles(expectedAlphaMonitoringNodesAfterLeaving).toList: _*) { awaitAssert({ @@ -180,16 +180,15 @@ abstract class MultiDcHeartbeatTakingOverSpec extends MultiNodeSpec(MultiDcHeart * strongly guaratnee the order of "oldest" members, as they're linearized by the order in which they become Up * (since marking that transition is a Leader action). */ - private def membersByAge(): immutable.SortedSet[Member] = + private def membersByAge(dataCenter: ClusterSettings.DataCenter): immutable.SortedSet[Member] = SortedSet.empty(Member.ageOrdering) - .union(cluster.state.members.filter(m ⇒ m.status != MemberStatus.Joining && m.status != MemberStatus.WeaklyUp)) + .union(cluster.state.members.filter(m ⇒ m.dataCenter == dataCenter && + m.status != MemberStatus.Joining && m.status != MemberStatus.WeaklyUp)) /** INTERNAL API */ @InternalApi - private[cluster] def takeNOldestMembers(memberFilter: Member ⇒ Boolean, n: Int): immutable.SortedSet[Member] = - membersByAge() - .filter(memberFilter) - .take(n) + private[cluster] def takeNOldestMembers(dataCenter: ClusterSettings.DataCenter, n: Int): immutable.SortedSet[Member] = + membersByAge(dataCenter).take(n) private def membersAsRoles(ms: SortedSet[Member]): SortedSet[RoleName] = { val res = ms.flatMap(m ⇒ roleName(m.address)) diff --git a/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcSunnyWeatherSpec.scala b/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcSunnyWeatherSpec.scala index e13f2bc691..9f340f31f9 100644 --- a/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcSunnyWeatherSpec.scala +++ b/akka-cluster/src/multi-jvm/scala/akka/cluster/MultiDcSunnyWeatherSpec.scala @@ -76,10 +76,10 @@ abstract class MultiDcSunnyWeatherSpec extends MultiNodeSpec(MultiDcSunnyWeather val crossDcHeartbeatSenderPath = "/system/cluster/core/daemon/crossDcHeartbeatSender" val selectCrossDcHeartbeatSender = system.actorSelection(crossDcHeartbeatSenderPath) - val expectedAlphaHeartbeaterNodes = takeNOldestMembers(_.dataCenter == "alpha", 2) + val expectedAlphaHeartbeaterNodes = takeNOldestMembers(dataCenter = "alpha", 2) val expectedAlphaHeartbeaterRoles = membersAsRoles(expectedAlphaHeartbeaterNodes) - val expectedBetaHeartbeaterNodes = takeNOldestMembers(_.dataCenter == "beta", 2) + val expectedBetaHeartbeaterNodes = takeNOldestMembers(dataCenter = "beta", 2) val expectedBetaHeartbeaterRoles = membersAsRoles(expectedBetaHeartbeaterNodes) val expectedNoActiveHeartbeatSenderRoles = roles.toSet -- (expectedAlphaHeartbeaterRoles union expectedBetaHeartbeaterRoles) @@ -147,16 +147,15 @@ abstract class MultiDcSunnyWeatherSpec extends MultiNodeSpec(MultiDcSunnyWeather * strongly guaratnee the order of "oldest" members, as they're linearized by the order in which they become Up * (since marking that transition is a Leader action). */ - private def membersByAge(): immutable.SortedSet[Member] = + private def membersByAge(dataCenter: ClusterSettings.DataCenter): immutable.SortedSet[Member] = SortedSet.empty(Member.ageOrdering) - .union(cluster.state.members.filter(m ⇒ m.status != MemberStatus.Joining && m.status != MemberStatus.WeaklyUp)) + .union(cluster.state.members.filter(m ⇒ m.dataCenter == dataCenter && + m.status != MemberStatus.Joining && m.status != MemberStatus.WeaklyUp)) /** INTERNAL API */ @InternalApi - private[cluster] def takeNOldestMembers(memberFilter: Member ⇒ Boolean, n: Int): immutable.SortedSet[Member] = - membersByAge() - .filter(memberFilter) - .take(n) + private[cluster] def takeNOldestMembers(dataCenter: ClusterSettings.DataCenter, n: Int): immutable.SortedSet[Member] = + membersByAge(dataCenter).take(n) private def membersAsRoles(ms: immutable.Set[Member]): immutable.Set[RoleName] = { val res = ms.flatMap(m ⇒ roleName(m.address))