Committed by
Gerrit Code Review
Eliminate the peformance penalty introduced in ReplicaInfoService.getReplicaInfoFor
Change-Id: Ie37d7e80c4dbf37a2ae6f452f6f66f4505d69a29
Showing
3 changed files
with
36 additions
and
3 deletions
| ... | @@ -22,6 +22,7 @@ import java.util.List; | ... | @@ -22,6 +22,7 @@ import java.util.List; |
| 22 | 22 | ||
| 23 | import org.onosproject.cluster.NodeId; | 23 | import org.onosproject.cluster.NodeId; |
| 24 | 24 | ||
| 25 | +import com.google.common.base.Objects; | ||
| 25 | import com.google.common.base.Optional; | 26 | import com.google.common.base.Optional; |
| 26 | 27 | ||
| 27 | /** | 28 | /** |
| ... | @@ -61,6 +62,21 @@ public final class ReplicaInfo { | ... | @@ -61,6 +62,21 @@ public final class ReplicaInfo { |
| 61 | return backups; | 62 | return backups; |
| 62 | } | 63 | } |
| 63 | 64 | ||
| 65 | + @Override | ||
| 66 | + public int hashCode() { | ||
| 67 | + return Objects.hashCode(master, backups); | ||
| 68 | + } | ||
| 69 | + | ||
| 70 | + @Override | ||
| 71 | + public boolean equals(Object other) { | ||
| 72 | + if (!(other instanceof ReplicaInfo)) { | ||
| 73 | + return false; | ||
| 74 | + } | ||
| 75 | + ReplicaInfo that = (ReplicaInfo) other; | ||
| 76 | + return Objects.equal(this.master, that.master) && | ||
| 77 | + Objects.equal(this.backups, that.backups); | ||
| 78 | + } | ||
| 79 | + | ||
| 64 | // for Serializer | 80 | // for Serializer |
| 65 | private ReplicaInfo() { | 81 | private ReplicaInfo() { |
| 66 | this.master = Optional.absent(); | 82 | this.master = Optional.absent(); | ... | ... |
| ... | @@ -22,6 +22,7 @@ import static org.onosproject.store.flow.ReplicaInfoEvent.Type.BACKUPS_CHANGED; | ... | @@ -22,6 +22,7 @@ import static org.onosproject.store.flow.ReplicaInfoEvent.Type.BACKUPS_CHANGED; |
| 22 | 22 | ||
| 23 | import java.util.Collections; | 23 | import java.util.Collections; |
| 24 | import java.util.List; | 24 | import java.util.List; |
| 25 | +import java.util.Map; | ||
| 25 | 26 | ||
| 26 | import org.apache.felix.scr.annotations.Activate; | 27 | import org.apache.felix.scr.annotations.Activate; |
| 27 | import org.apache.felix.scr.annotations.Component; | 28 | import org.apache.felix.scr.annotations.Component; |
| ... | @@ -43,6 +44,10 @@ import org.onosproject.store.flow.ReplicaInfoEventListener; | ... | @@ -43,6 +44,10 @@ import org.onosproject.store.flow.ReplicaInfoEventListener; |
| 43 | import org.onosproject.store.flow.ReplicaInfoService; | 44 | import org.onosproject.store.flow.ReplicaInfoService; |
| 44 | import org.slf4j.Logger; | 45 | import org.slf4j.Logger; |
| 45 | 46 | ||
| 47 | +import com.google.common.base.Objects; | ||
| 48 | +import com.google.common.collect.ImmutableList; | ||
| 49 | +import com.google.common.collect.Maps; | ||
| 50 | + | ||
| 46 | /** | 51 | /** |
| 47 | * Manages replica placement information. | 52 | * Manages replica placement information. |
| 48 | */ | 53 | */ |
| ... | @@ -63,6 +68,8 @@ public class ReplicaInfoManager implements ReplicaInfoService { | ... | @@ -63,6 +68,8 @@ public class ReplicaInfoManager implements ReplicaInfoService { |
| 63 | protected final AbstractListenerRegistry<ReplicaInfoEvent, ReplicaInfoEventListener> | 68 | protected final AbstractListenerRegistry<ReplicaInfoEvent, ReplicaInfoEventListener> |
| 64 | listenerRegistry = new AbstractListenerRegistry<>(); | 69 | listenerRegistry = new AbstractListenerRegistry<>(); |
| 65 | 70 | ||
| 71 | + private final Map<DeviceId, ReplicaInfo> deviceReplicaInfoMap = Maps.newConcurrentMap(); | ||
| 72 | + | ||
| 66 | @Activate | 73 | @Activate |
| 67 | public void activate() { | 74 | public void activate() { |
| 68 | eventDispatcher.addSink(ReplicaInfoEvent.class, listenerRegistry); | 75 | eventDispatcher.addSink(ReplicaInfoEvent.class, listenerRegistry); |
| ... | @@ -79,7 +86,9 @@ public class ReplicaInfoManager implements ReplicaInfoService { | ... | @@ -79,7 +86,9 @@ public class ReplicaInfoManager implements ReplicaInfoService { |
| 79 | 86 | ||
| 80 | @Override | 87 | @Override |
| 81 | public ReplicaInfo getReplicaInfoFor(DeviceId deviceId) { | 88 | public ReplicaInfo getReplicaInfoFor(DeviceId deviceId) { |
| 82 | - return buildFromRoleInfo(mastershipService.getNodesFor(deviceId)); | 89 | + return deviceReplicaInfoMap.computeIfAbsent( |
| 90 | + deviceId, | ||
| 91 | + id -> buildFromRoleInfo(mastershipService.getNodesFor(deviceId))); | ||
| 83 | } | 92 | } |
| 84 | 93 | ||
| 85 | @Override | 94 | @Override |
| ... | @@ -94,7 +103,7 @@ public class ReplicaInfoManager implements ReplicaInfoService { | ... | @@ -94,7 +103,7 @@ public class ReplicaInfoManager implements ReplicaInfoService { |
| 94 | 103 | ||
| 95 | private static ReplicaInfo buildFromRoleInfo(RoleInfo roles) { | 104 | private static ReplicaInfo buildFromRoleInfo(RoleInfo roles) { |
| 96 | List<NodeId> backups = roles.backups() == null ? | 105 | List<NodeId> backups = roles.backups() == null ? |
| 97 | - Collections.emptyList() : roles.backups(); | 106 | + Collections.emptyList() : ImmutableList.copyOf(roles.backups()); |
| 98 | return new ReplicaInfo(roles.master(), backups); | 107 | return new ReplicaInfo(roles.master(), backups); |
| 99 | } | 108 | } |
| 100 | 109 | ||
| ... | @@ -102,8 +111,12 @@ public class ReplicaInfoManager implements ReplicaInfoService { | ... | @@ -102,8 +111,12 @@ public class ReplicaInfoManager implements ReplicaInfoService { |
| 102 | 111 | ||
| 103 | @Override | 112 | @Override |
| 104 | public void event(MastershipEvent event) { | 113 | public void event(MastershipEvent event) { |
| 114 | + final DeviceId deviceId = event.subject(); | ||
| 105 | final ReplicaInfo replicaInfo = buildFromRoleInfo(event.roleInfo()); | 115 | final ReplicaInfo replicaInfo = buildFromRoleInfo(event.roleInfo()); |
| 106 | - | 116 | + ReplicaInfo oldReplicaInfo = deviceReplicaInfoMap.put(deviceId, replicaInfo); |
| 117 | + if (Objects.equal(oldReplicaInfo, replicaInfo)) { | ||
| 118 | + return; | ||
| 119 | + } | ||
| 107 | switch (event.type()) { | 120 | switch (event.type()) { |
| 108 | case MASTER_CHANGED: | 121 | case MASTER_CHANGED: |
| 109 | eventDispatcher.post(new ReplicaInfoEvent(MASTER_CHANGED, | 122 | eventDispatcher.post(new ReplicaInfoEvent(MASTER_CHANGED, | ... | ... |
| ... | @@ -17,6 +17,7 @@ package org.onosproject.store.mastership.impl; | ... | @@ -17,6 +17,7 @@ package org.onosproject.store.mastership.impl; |
| 17 | 17 | ||
| 18 | import static org.onlab.util.Tools.groupedThreads; | 18 | import static org.onlab.util.Tools.groupedThreads; |
| 19 | import static org.onlab.util.Tools.futureGetOrElse; | 19 | import static org.onlab.util.Tools.futureGetOrElse; |
| 20 | +import static org.onosproject.mastership.MastershipEvent.Type.BACKUPS_CHANGED; | ||
| 20 | import static org.onosproject.mastership.MastershipEvent.Type.MASTER_CHANGED; | 21 | import static org.onosproject.mastership.MastershipEvent.Type.MASTER_CHANGED; |
| 21 | import static org.slf4j.LoggerFactory.getLogger; | 22 | import static org.slf4j.LoggerFactory.getLogger; |
| 22 | import static com.google.common.base.Preconditions.checkArgument; | 23 | import static com.google.common.base.Preconditions.checkArgument; |
| ... | @@ -409,6 +410,9 @@ public class ConsistentDeviceMastershipStore | ... | @@ -409,6 +410,9 @@ public class ConsistentDeviceMastershipStore |
| 409 | case LEADER_BOOTED: | 410 | case LEADER_BOOTED: |
| 410 | notifyDelegate(new MastershipEvent(MASTER_CHANGED, deviceId, getNodes(deviceId))); | 411 | notifyDelegate(new MastershipEvent(MASTER_CHANGED, deviceId, getNodes(deviceId))); |
| 411 | break; | 412 | break; |
| 413 | + case CANDIDATES_CHANGED: | ||
| 414 | + notifyDelegate(new MastershipEvent(BACKUPS_CHANGED, deviceId, getNodes(deviceId))); | ||
| 415 | + break; | ||
| 412 | default: | 416 | default: |
| 413 | return; | 417 | return; |
| 414 | } | 418 | } | ... | ... |
-
Please register or login to post a comment