KryoNamespace improvements

- Ignore duplicate Namespace registration - Add friendly name for helping logging - ONOS-4528 Change-Id: Id78f2a0f6e9715a7880875039825e294a68592a9

KryoNamespace improvements
- Ignore duplicate Namespace registration - Add friendly name for helping logging - ONOS-4528 Change-Id: Id78f2a0f6e9715a7880875039825e294a68592a9
HIGUCHI Yuta · Gerrit Code Review
Commit b2d0fd83808ea036dc4b7f3a88e68a2810942517 b2d0fd83 1 parent 38eeac48
Showing 2 changed files with 67 additions and 11 deletions
core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
utils/misc/src/main/java/org/onlab/util/KryoNamespace.java
--- a/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
View file @b2d0fd8
+++ b/core/store/serializers/src/main/java/org/onosproject/store/serializers/KryoNamespaces.java
View file @b2d0fd8
@@ -277,7 +277,7 @@ public final class KryoNamespaces {
             .register(char[].class)
             .register(String[].class)
             .register(boolean[].class)
-            .build();
+            .build("BASIC");
     /**
      * KryoNamespace which can serialize ON.lab misc classes.
@@ -297,7 +297,7 @@ public final class KryoNamespaces {
             .register(Bandwidth.class)
             .register(Bandwidth.bps(1L).getClass())
             .register(Bandwidth.bps(1.0).getClass())
-            .build();
+            .build("MISC");
     /**
      * Kryo registration Id for user custom registration.
@@ -542,7 +542,7 @@ public final class KryoNamespaces {
             .register(VlanCodec.class)
             .register(MplsCodec.class)
             .register(NoOpCodec.class)
-            .build();
+            .build("API");
     // not to be instantiated
--- a/utils/misc/src/main/java/org/onlab/util/KryoNamespace.java
View file @b2d0fd8
+++ b/utils/misc/src/main/java/org/onlab/util/KryoNamespace.java
View file @b2d0fd8
@@ -35,7 +35,9 @@ import java.io.OutputStream;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 /**
@@ -62,9 +64,12 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
      */
     public static final int INITIAL_ID = 16;
+    private static final String NO_NAME = "(no name)";
+
     private static final Logger log = getLogger(KryoNamespace.class);
+
     private final KryoPool pool = new KryoPool.Builder(this)
                                         .softReferences()
                                         .build();
@@ -72,6 +77,7 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
     private final ImmutableList<RegistrationBlock> registeredBlocks;
     private final boolean registrationRequired;
+    private final String friendlyName;
     /**
@@ -91,10 +97,20 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
          * @return KryoNamespace
          */
         public KryoNamespace build() {
+            return build(NO_NAME);
+        }
+
+        /**
+         * Builds a {@link KryoNamespace} instance.
+         *
+         * @param friendlyName friendly name for the namespace
+         * @return KryoNamespace
+         */
+        public KryoNamespace build(String friendlyName) {
             if (!types.isEmpty()) {
                 blocks.add(new RegistrationBlock(this.blockHeadId, types));
             }
-            return new KryoNamespace(blocks, registrationRequired).populate(1);
+            return new KryoNamespace(blocks, registrationRequired, friendlyName).populate(1);
         }
         /**
@@ -109,7 +125,7 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
             if (!types.isEmpty()) {
                 if (id != FLOATING_ID && id < blockHeadId + types.size()) {
-                    log.warn("requested nextId {} could potentially overlap" +
+                    log.warn("requested nextId {} could potentially overlap " +
                              "with existing registrations {}+{} ",
                              id, blockHeadId, types.size());
                 }
@@ -170,6 +186,12 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
          * @return this
          */
         public Builder register(final KryoNamespace ns) {
+
+            if (blocks.containsAll(ns.registeredBlocks)) {
+                // Everything was already registered.
+                log.debug("Ignoring {}, already registered.", ns);
+                return this;
+            }
             for (RegistrationBlock block : ns.registeredBlocks) {
                 this.register(block);
             }
@@ -204,10 +226,14 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
      *
      * @param registeredTypes types to register
      * @param registrationRequired
+     * @param friendlyName friendly name for the namespace
      */
-    private KryoNamespace(final List<RegistrationBlock> registeredTypes, boolean registrationRequired) {
+    private KryoNamespace(final List<RegistrationBlock> registeredTypes,
+                          boolean registrationRequired,
+                          String friendlyName) {
         this.registeredBlocks = ImmutableList.copyOf(registeredTypes);
         this.registrationRequired = registrationRequired;
+        this.friendlyName =  checkNotNull(friendlyName);
     }
     /**
@@ -373,6 +399,10 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
         }
     }
+    private String friendlyName() {
+        return friendlyName;
+    }
+
     /**
      * Creates a Kryo instance.
      *
@@ -408,12 +438,12 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
      * @param serializer Specific serializer to register or null to use default.
      * @param id         type registration id to use
      */
-    private static void register(Kryo kryo, Class<?> type, Serializer<?> serializer, int id) {
+    private void register(Kryo kryo, Class<?> type, Serializer<?> serializer, int id) {
         Registration existing = kryo.getRegistration(id);
         if (existing != null) {
             if (existing.getType() != type) {
-                log.error("Failed to register {} as {}, {} was already registered.",
+                log.error("{}: Failed to register {} as {}, {} was already registered.",
-                          type, id, existing.getType());
+                          friendlyName(), type, id, existing.getType());
                 throw new IllegalStateException(String.format(
                           "Failed to register %s as %s, %s was already registered.",
@@ -430,8 +460,8 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
             r = kryo.register(type, serializer, id);
         }
         if (r.getId() != id) {
-            log.debug("{} already registed as {}. Skipping {}.",
+            log.warn("{}: {} already registed as {}. Skipping {}.",
-                     r.getType(), r.getId(), id);
+                     friendlyName(), r.getType(), r.getId(), id);
         }
         log.trace("{} registered as {}", r.getType(), r.getId());
     }
@@ -453,6 +483,13 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
     @Override
     public String toString() {
+        if (friendlyName != NO_NAME) {
+            return MoreObjects.toStringHelper(getClass())
+                    .omitNullValues()
+                    .add("friendlyName", friendlyName)
+                    // omit lengthy detail, when there's a name
+                    .toString();
+        }
         return MoreObjects.toStringHelper(getClass())
                     .add("registeredBlocks", registeredBlocks)
                     .toString();
@@ -482,5 +519,24 @@ public final class KryoNamespace implements KryoFactory, KryoPool {
                     .add("types", types)
                     .toString();
         }
+
+        @Override
+        public int hashCode() {
+            return types.hashCode();
+        }
+
+        // Only the registered types are used for equality.
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+
+            if (obj instanceof RegistrationBlock) {
+                RegistrationBlock that = (RegistrationBlock) obj;
+                return Objects.equals(this.types, that.types);
+            }
+            return false;
+        }
     }
 }