Only update ZKObject values in ZK if necessary

Several parts of the NNFI implementation set attributes without
checking their existing values.  Usually they will be the same
from one pass to the next (like "item.active = active").  That
is inefficient.  Rather than updating all that code and changing
our python programming habits, let's make ZKObject smarter so it
doesn't write to ZK when it's not necessary.

Change-Id: Ibbe0d52a0f6ce28ad26753f7c5133027f572b33c

1 files changed, 34 insertions, 31 deletions

diff --git a/zuul/zk/zkobject.py b/zuul/zk/zkobject.py
index 20190e9ef..7e09108be 100644
--- a/zuul/zk/zkobject.py
+++ b/zuul/zk/zkobject.py
@@ -98,13 +98,16 @@ class ZKObject:
         call as possible for efficient network use.
         """
         old = self.__dict__.copy()
+        oldserial = self._trySerialize(context)
         self._set(**kw)
-        try:
-            self._save(context)
-        except Exception:
-            # Roll back our old values if we aren't able to update ZK.
-            self._set(**old)
-            raise
+        newserial = self._trySerialize(context)
+        if oldserial != newserial:
+            try:
+                self._save(context, newserial)
+            except Exception:
+                # Roll back our old values if we aren't able to update ZK.
+                self._set(**old)
+                raise
 
     @contextlib.contextmanager
     def activeContext(self, context):
@@ -113,14 +116,17 @@ class ZKObject:
                 f"Another context is already active {self._active_context}")
         try:
             old = self.__dict__.copy()
+            oldserial = self._trySerialize(context)
             self._set(_active_context=context)
             yield
-            try:
-                self._save(context)
-            except Exception:
-                # Roll back our old values if we aren't able to update ZK.
-                self._set(**old)
-                raise
+            newserial = self._trySerialize(context)
+            if oldserial != newserial:
+                try:
+                    self._save(context, newserial)
+                except Exception:
+                    # Roll back our old values if we aren't able to update ZK.
+                    self._set(**old)
+                    raise
         finally:
             self._set(_active_context=None)
 
@@ -129,7 +135,8 @@ class ZKObject:
         """Create a new instance and save it in ZooKeeper"""
         obj = klass()
         obj._set(**kw)
-        obj._save(context, create=True)
+        data = obj._trySerialize(context)
+        obj._save(context, data, create=True)
         return obj
 
     @classmethod
@@ -144,6 +151,18 @@ class ZKObject:
         """Update data from ZK"""
         self._load(context)
 
+    def _trySerialize(self, context):
+        if isinstance(context, LocalZKContext):
+            return b''
+        try:
+            return self.serialize()
+        except Exception:
+            # A higher level must handle this exception, but log
+            # ourself here so we know what object triggered it.
+            context.log.error(
+                "Exception serializing ZKObject %s", self)
+            raise
+
     def delete(self, context):
         path = self.getPath()
         while context.sessionIsValid():
@@ -195,17 +214,9 @@ class ZKObject:
                 raise
         raise Exception("ZooKeeper session or lock not valid")
 
-    def _save(self, context, create=False):
+    def _save(self, context, data, create=False):
         if isinstance(context, LocalZKContext):
             return
-        try:
-            data = self.serialize()
-        except Exception:
-            # A higher level must handle this exception, but log
-            # ourself here so we know what object triggered it.
-            context.log.error(
-                "Exception serializing ZKObject %s", self)
-            raise
         path = self.getPath()
         while context.sessionIsValid():
             try:
@@ -275,17 +286,9 @@ class ShardedZKObject(ZKObject):
                 raise InvalidObjectError from exc
         raise Exception("ZooKeeper session or lock not valid")
 
-    def _save(self, context, create=False):
+    def _save(self, context, data, create=False):
         if isinstance(context, LocalZKContext):
             return
-        try:
-            data = self.serialize()
-        except Exception:
-            # A higher level must handle this exception, but log
-            # ourself here so we know what object triggered it.
-            context.log.error(
-                "Exception serializing ZKObject %s", self)
-            raise
         path = self.getPath()
         while context.sessionIsValid():
             try: