Committed by
Gerrit Code Review
Some improvements around EventuallyConsistentMapBuilder serializer handling.
- Allow directly passing KryoNamespace - Add some registration id gap before ECMap's internal registration - Some improvements for ease of registration issue investigation -- Add friendly name to ECMap's internal KryoNamespace -- Add backtrace information Change-Id: I7c87b3aefbaea4b2ed12b38c3e0813e9d195c7a9
Showing
7 changed files
with
54 additions
and
24 deletions
... | @@ -182,6 +182,11 @@ public final class VtnEventuallyConsistentMapTest<K, V> extends VtnEventuallyCon | ... | @@ -182,6 +182,11 @@ public final class VtnEventuallyConsistentMapTest<K, V> extends VtnEventuallyCon |
182 | } | 182 | } |
183 | 183 | ||
184 | @Override | 184 | @Override |
185 | + public EventuallyConsistentMapBuilder<K, V> withSerializer(KryoNamespace serializer) { | ||
186 | + return this; | ||
187 | + } | ||
188 | + | ||
189 | + @Override | ||
185 | public EventuallyConsistentMapBuilder<K, V> | 190 | public EventuallyConsistentMapBuilder<K, V> |
186 | withTimestampProvider(BiFunction<K, V, Timestamp> timestampProvider) { | 191 | withTimestampProvider(BiFunction<K, V, Timestamp> timestampProvider) { |
187 | return this; | 192 | return this; | ... | ... |
... | @@ -67,6 +67,20 @@ public interface EventuallyConsistentMapBuilder<K, V> { | ... | @@ -67,6 +67,20 @@ public interface EventuallyConsistentMapBuilder<K, V> { |
67 | KryoNamespace.Builder serializerBuilder); | 67 | KryoNamespace.Builder serializerBuilder); |
68 | 68 | ||
69 | /** | 69 | /** |
70 | + * Sets a serializer that can be used to create a serializer that | ||
71 | + * can serialize both the keys and values put into the map. The serializer | ||
72 | + * builder should be pre-populated with any classes that will be put into | ||
73 | + * the map. | ||
74 | + * <p> | ||
75 | + * Note: This is a mandatory parameter. | ||
76 | + * </p> | ||
77 | + * | ||
78 | + * @param serializer serializer | ||
79 | + * @return this EventuallyConsistentMapBuilder | ||
80 | + */ | ||
81 | + EventuallyConsistentMapBuilder<K, V> withSerializer(KryoNamespace serializer); | ||
82 | + | ||
83 | + /** | ||
70 | * Sets the function to use for generating timestamps for map updates. | 84 | * Sets the function to use for generating timestamps for map updates. |
71 | * <p> | 85 | * <p> |
72 | * The client must provide an {@code BiFunction<K, V, Timestamp>} | 86 | * The client must provide an {@code BiFunction<K, V, Timestamp>} | ... | ... |
... | @@ -179,6 +179,11 @@ public final class TestEventuallyConsistentMap<K, V> extends EventuallyConsisten | ... | @@ -179,6 +179,11 @@ public final class TestEventuallyConsistentMap<K, V> extends EventuallyConsisten |
179 | } | 179 | } |
180 | 180 | ||
181 | @Override | 181 | @Override |
182 | + public EventuallyConsistentMapBuilder<K, V> withSerializer(KryoNamespace serializer) { | ||
183 | + return this; | ||
184 | + } | ||
185 | + | ||
186 | + @Override | ||
182 | public EventuallyConsistentMapBuilder<K, V> | 187 | public EventuallyConsistentMapBuilder<K, V> |
183 | withTimestampProvider(BiFunction<K, V, Timestamp> timestampProvider) { | 188 | withTimestampProvider(BiFunction<K, V, Timestamp> timestampProvider) { |
184 | return this; | 189 | return this; | ... | ... |
... | @@ -42,6 +42,7 @@ public class EventuallyConsistentMapBuilderImpl<K, V> | ... | @@ -42,6 +42,7 @@ public class EventuallyConsistentMapBuilderImpl<K, V> |
42 | private final ClusterCommunicationService clusterCommunicator; | 42 | private final ClusterCommunicationService clusterCommunicator; |
43 | 43 | ||
44 | private String name; | 44 | private String name; |
45 | + private KryoNamespace serializer; | ||
45 | private KryoNamespace.Builder serializerBuilder; | 46 | private KryoNamespace.Builder serializerBuilder; |
46 | private ExecutorService eventExecutor; | 47 | private ExecutorService eventExecutor; |
47 | private ExecutorService communicationExecutor; | 48 | private ExecutorService communicationExecutor; |
... | @@ -85,6 +86,12 @@ public class EventuallyConsistentMapBuilderImpl<K, V> | ... | @@ -85,6 +86,12 @@ public class EventuallyConsistentMapBuilderImpl<K, V> |
85 | } | 86 | } |
86 | 87 | ||
87 | @Override | 88 | @Override |
89 | + public EventuallyConsistentMapBuilder<K, V> withSerializer(KryoNamespace serializer) { | ||
90 | + this.serializer = checkNotNull(serializer); | ||
91 | + return this; | ||
92 | + } | ||
93 | + | ||
94 | + @Override | ||
88 | public EventuallyConsistentMapBuilder<K, V> withTimestampProvider( | 95 | public EventuallyConsistentMapBuilder<K, V> withTimestampProvider( |
89 | BiFunction<K, V, Timestamp> timestampProvider) { | 96 | BiFunction<K, V, Timestamp> timestampProvider) { |
90 | this.timestampProvider = checkNotNull(timestampProvider); | 97 | this.timestampProvider = checkNotNull(timestampProvider); |
... | @@ -147,13 +154,16 @@ public class EventuallyConsistentMapBuilderImpl<K, V> | ... | @@ -147,13 +154,16 @@ public class EventuallyConsistentMapBuilderImpl<K, V> |
147 | @Override | 154 | @Override |
148 | public EventuallyConsistentMap<K, V> build() { | 155 | public EventuallyConsistentMap<K, V> build() { |
149 | checkNotNull(name, "name is a mandatory parameter"); | 156 | checkNotNull(name, "name is a mandatory parameter"); |
150 | - checkNotNull(serializerBuilder, "serializerBuilder is a mandatory parameter"); | ||
151 | checkNotNull(timestampProvider, "timestampProvider is a mandatory parameter"); | 157 | checkNotNull(timestampProvider, "timestampProvider is a mandatory parameter"); |
158 | + if (serializer == null && serializerBuilder != null) { | ||
159 | + serializer = serializerBuilder.build(name); | ||
160 | + } | ||
161 | + checkNotNull(serializer, "serializer is a mandatory parameter"); | ||
152 | 162 | ||
153 | return new EventuallyConsistentMapImpl<>(name, | 163 | return new EventuallyConsistentMapImpl<>(name, |
154 | clusterService, | 164 | clusterService, |
155 | clusterCommunicator, | 165 | clusterCommunicator, |
156 | - serializerBuilder, | 166 | + serializer, |
157 | timestampProvider, | 167 | timestampProvider, |
158 | peerUpdateFunction, | 168 | peerUpdateFunction, |
159 | eventExecutor, | 169 | eventExecutor, | ... | ... |
... | @@ -58,7 +58,6 @@ import org.onosproject.store.serializers.StoreSerializer; | ... | @@ -58,7 +58,6 @@ import org.onosproject.store.serializers.StoreSerializer; |
58 | import org.onosproject.store.service.EventuallyConsistentMap; | 58 | import org.onosproject.store.service.EventuallyConsistentMap; |
59 | import org.onosproject.store.service.EventuallyConsistentMapEvent; | 59 | import org.onosproject.store.service.EventuallyConsistentMapEvent; |
60 | import org.onosproject.store.service.EventuallyConsistentMapListener; | 60 | import org.onosproject.store.service.EventuallyConsistentMapListener; |
61 | -import org.onosproject.store.service.Serializer; | ||
62 | import org.onosproject.store.service.WallClockTimestamp; | 61 | import org.onosproject.store.service.WallClockTimestamp; |
63 | import org.slf4j.Logger; | 62 | import org.slf4j.Logger; |
64 | import org.slf4j.LoggerFactory; | 63 | import org.slf4j.LoggerFactory; |
... | @@ -138,7 +137,7 @@ public class EventuallyConsistentMapImpl<K, V> | ... | @@ -138,7 +137,7 @@ public class EventuallyConsistentMapImpl<K, V> |
138 | * @param mapName a String identifier for the map. | 137 | * @param mapName a String identifier for the map. |
139 | * @param clusterService the cluster service | 138 | * @param clusterService the cluster service |
140 | * @param clusterCommunicator the cluster communications service | 139 | * @param clusterCommunicator the cluster communications service |
141 | - * @param serializerBuilder a Kryo namespace builder that can serialize | 140 | + * @param serializer a Kryo namespace that can serialize |
142 | * both K and V | 141 | * both K and V |
143 | * @param timestampProvider provider of timestamps for K and V | 142 | * @param timestampProvider provider of timestamps for K and V |
144 | * @param peerUpdateFunction function that provides a set of nodes to immediately | 143 | * @param peerUpdateFunction function that provides a set of nodes to immediately |
... | @@ -159,7 +158,7 @@ public class EventuallyConsistentMapImpl<K, V> | ... | @@ -159,7 +158,7 @@ public class EventuallyConsistentMapImpl<K, V> |
159 | EventuallyConsistentMapImpl(String mapName, | 158 | EventuallyConsistentMapImpl(String mapName, |
160 | ClusterService clusterService, | 159 | ClusterService clusterService, |
161 | ClusterCommunicationService clusterCommunicator, | 160 | ClusterCommunicationService clusterCommunicator, |
162 | - KryoNamespace.Builder serializerBuilder, | 161 | + KryoNamespace ns, |
163 | BiFunction<K, V, Timestamp> timestampProvider, | 162 | BiFunction<K, V, Timestamp> timestampProvider, |
164 | BiFunction<K, V, Collection<NodeId>> peerUpdateFunction, | 163 | BiFunction<K, V, Collection<NodeId>> peerUpdateFunction, |
165 | ExecutorService eventExecutor, | 164 | ExecutorService eventExecutor, |
... | @@ -172,25 +171,14 @@ public class EventuallyConsistentMapImpl<K, V> | ... | @@ -172,25 +171,14 @@ public class EventuallyConsistentMapImpl<K, V> |
172 | boolean persistent, | 171 | boolean persistent, |
173 | PersistenceService persistenceService) { | 172 | PersistenceService persistenceService) { |
174 | this.mapName = mapName; | 173 | this.mapName = mapName; |
175 | - this.serializer = createSerializer(serializerBuilder); | 174 | + this.serializer = createSerializer(ns); |
176 | this.persistenceService = persistenceService; | 175 | this.persistenceService = persistenceService; |
177 | this.persistent = | 176 | this.persistent = |
178 | persistent; | 177 | persistent; |
179 | if (persistent) { | 178 | if (persistent) { |
180 | items = this.persistenceService.<K, MapValue<V>>persistentMapBuilder() | 179 | items = this.persistenceService.<K, MapValue<V>>persistentMapBuilder() |
181 | .withName(PERSISTENT_LOCAL_MAP_NAME) | 180 | .withName(PERSISTENT_LOCAL_MAP_NAME) |
182 | - .withSerializer(new Serializer() { | 181 | + .withSerializer(this.serializer) |
183 | - | ||
184 | - @Override | ||
185 | - public <T> byte[] encode(T object) { | ||
186 | - return EventuallyConsistentMapImpl.this.serializer.encode(object); | ||
187 | - } | ||
188 | - | ||
189 | - @Override | ||
190 | - public <T> T decode(byte[] bytes) { | ||
191 | - return EventuallyConsistentMapImpl.this.serializer.decode(bytes); | ||
192 | - } | ||
193 | - }) | ||
194 | .build(); | 182 | .build(); |
195 | } else { | 183 | } else { |
196 | items = Maps.newConcurrentMap(); | 184 | items = Maps.newConcurrentMap(); |
... | @@ -268,10 +256,13 @@ public class EventuallyConsistentMapImpl<K, V> | ... | @@ -268,10 +256,13 @@ public class EventuallyConsistentMapImpl<K, V> |
268 | this.lightweightAntiEntropy = !convergeFaster; | 256 | this.lightweightAntiEntropy = !convergeFaster; |
269 | } | 257 | } |
270 | 258 | ||
271 | - private StoreSerializer createSerializer(KryoNamespace.Builder builder) { | 259 | + private StoreSerializer createSerializer(KryoNamespace ns) { |
272 | - return StoreSerializer.using(builder | 260 | + return StoreSerializer.using(KryoNamespace.newBuilder() |
261 | + .register(ns) | ||
262 | + // not so robust way to avoid collision with other | ||
263 | + // user supplied registrations | ||
264 | + .nextId(KryoNamespaces.BEGIN_USER_CUSTOM_ID + 100) | ||
273 | .register(KryoNamespaces.BASIC) | 265 | .register(KryoNamespaces.BASIC) |
274 | - .nextId(KryoNamespaces.BEGIN_USER_CUSTOM_ID) | ||
275 | .register(LogicalTimestamp.class) | 266 | .register(LogicalTimestamp.class) |
276 | .register(WallClockTimestamp.class) | 267 | .register(WallClockTimestamp.class) |
277 | .register(AntiEntropyAdvertisement.class) | 268 | .register(AntiEntropyAdvertisement.class) |
... | @@ -279,7 +270,7 @@ public class EventuallyConsistentMapImpl<K, V> | ... | @@ -279,7 +270,7 @@ public class EventuallyConsistentMapImpl<K, V> |
279 | .register(UpdateEntry.class) | 270 | .register(UpdateEntry.class) |
280 | .register(MapValue.class) | 271 | .register(MapValue.class) |
281 | .register(MapValue.Digest.class) | 272 | .register(MapValue.Digest.class) |
282 | - .build(name())); | 273 | + .build(name() + "-ecmap")); |
283 | } | 274 | } |
284 | 275 | ||
285 | @Override | 276 | @Override | ... | ... |
... | @@ -20,11 +20,12 @@ import java.io.OutputStream; | ... | @@ -20,11 +20,12 @@ import java.io.OutputStream; |
20 | import java.nio.ByteBuffer; | 20 | import java.nio.ByteBuffer; |
21 | 21 | ||
22 | import org.onlab.util.KryoNamespace; | 22 | import org.onlab.util.KryoNamespace; |
23 | +import org.onosproject.store.service.Serializer; | ||
23 | 24 | ||
24 | /** | 25 | /** |
25 | * Service to serialize Objects into byte array. | 26 | * Service to serialize Objects into byte array. |
26 | */ | 27 | */ |
27 | -public interface StoreSerializer { | 28 | +public interface StoreSerializer extends Serializer { |
28 | 29 | ||
29 | /** | 30 | /** |
30 | * Serializes the specified object into bytes. | 31 | * Serializes the specified object into bytes. |
... | @@ -32,6 +33,7 @@ public interface StoreSerializer { | ... | @@ -32,6 +33,7 @@ public interface StoreSerializer { |
32 | * @param obj object to be serialized | 33 | * @param obj object to be serialized |
33 | * @return serialized bytes | 34 | * @return serialized bytes |
34 | */ | 35 | */ |
36 | + @Override | ||
35 | byte[] encode(final Object obj); | 37 | byte[] encode(final Object obj); |
36 | 38 | ||
37 | /** | 39 | /** |
... | @@ -57,6 +59,7 @@ public interface StoreSerializer { | ... | @@ -57,6 +59,7 @@ public interface StoreSerializer { |
57 | * @return deserialized object | 59 | * @return deserialized object |
58 | * @param <T> decoded type | 60 | * @param <T> decoded type |
59 | */ | 61 | */ |
62 | + @Override | ||
60 | <T> T decode(final byte[] bytes); | 63 | <T> T decode(final byte[] bytes); |
61 | 64 | ||
62 | /** | 65 | /** | ... | ... |
... | @@ -125,9 +125,11 @@ public final class KryoNamespace implements KryoFactory, KryoPool { | ... | @@ -125,9 +125,11 @@ public final class KryoNamespace implements KryoFactory, KryoPool { |
125 | if (!types.isEmpty()) { | 125 | if (!types.isEmpty()) { |
126 | if (id != FLOATING_ID && id < blockHeadId + types.size()) { | 126 | if (id != FLOATING_ID && id < blockHeadId + types.size()) { |
127 | 127 | ||
128 | + if (log.isWarnEnabled()) { | ||
128 | log.warn("requested nextId {} could potentially overlap " + | 129 | log.warn("requested nextId {} could potentially overlap " + |
129 | "with existing registrations {}+{} ", | 130 | "with existing registrations {}+{} ", |
130 | - id, blockHeadId, types.size()); | 131 | + id, blockHeadId, types.size(), new RuntimeException()); |
132 | + } | ||
131 | } | 133 | } |
132 | blocks.add(new RegistrationBlock(this.blockHeadId, types)); | 134 | blocks.add(new RegistrationBlock(this.blockHeadId, types)); |
133 | types = new ArrayList<>(); | 135 | types = new ArrayList<>(); | ... | ... |
-
Please register or login to post a comment