[2136] renamed 'pid' to 'mid' and change the default value -1 to None on

the second argument of the the 'update_statistics_data' method

Instead of PID(Process Id) used in previous version of stats codes,
'mid' (module id) is used for identifying multiple instances of same
module. 'lname' of CC-session in stats codes is assigned to it.

src/bin/stats/stats.py.in

@@ -129,8 +129,8 @@ class Stats:
         self.module_name = self.mccs.get_module_spec().get_module_name()
         self.modules = {}
         self.statistics_data = {}
-        # statistics data by each pid
-        self.statistics_data_bypid = {}
+        # statistics data by each mid
+        self.statistics_data_bymid = {}
         # get commands spec
         self.commands_spec = self.mccs.get_module_spec().get_commands_spec()
         # add event handler related command_handler of ModuleCCSession
@@ -211,7 +211,7 @@ class Stats:
                             rcode, args = isc.config.ccsession.parse_answer(answer)
                         if rcode == 0:
                             errors = self.update_statistics_data(
-                                module_name, env['from'] if n > 1 else -1, **args)
+                                module_name, env['from'], **args)
                             if errors:
                                 raise StatsError("spec file is incorrect: "
                                                  + ", ".join(errors))
@@ -327,14 +327,14 @@ class Stats:
                          + "owner: " + str(owner) + ", "
                          + "name: " + str(name))
 
-    def update_statistics_data(self, owner=None, pid=-1, **data):
+    def update_statistics_data(self, owner=None, mid=None, **data):
         """
         change statistics date of specified module into specified
         data. It updates information of each module first, and it
         updates statistics data. If specified data is invalid for
         statistics spec of specified owner, it returns a list of error
         messages. If there is no error or if neither owner nor data is
-        specified in args, it returns None. pid is the process id of
+        specified in args, it returns None. The 'mid' argument is a identifier of
         the sender module in order for stats to identify which
         instance sends statistics data in the situation that multiple
         instances are working.
@@ -342,16 +342,16 @@ class Stats:
         # Note:
         # The fix of #1751 is for multiple instances working. It is
         # assumed here that they send different statistics data with
-        # each PID. Stats should save their statistics data by
-        # PID. The statistics data, which is the existing variable, is
-        # preserved by accumlating from statistics data by PID. This
+        # each the sender module id(mid) . Stats should save their statistics data by
+        # mid. The statistics data, which is the existing variable, is
+        # preserved by accumlating from statistics data by the module id. This
         # is an ad-hoc fix because administrators can not see
         # statistics by each instance via bindctl or HTTP/XML. These
         # interfaces aren't changed in this fix.
 
-        def _accum_bymodule(statistics_data_bypid):
+        def _accum_bymodule(statistics_data_bymid):
             # This is an internal function for the superordinate
-            # function. It accumulates statistics data of each PID by
+            # function. It accumulates statistics data of each module id by
             # module. It returns the accumulation result.
             def _accum(a, b):
                 # If the first arg is dict or list type, two values
@@ -372,6 +372,12 @@ class Stats:
                                          if len(a) <= i ]
                 # If the first arg is integer or float type, two
                 # values are just added.
+                # FIXME: A issue might happen when consolidating
+                # statistics of the multiple instances. If they have
+                # different statistics data which are not for adding
+                # each other, this might happen: If these are integer
+                # or float, these are added each other, otherwise
+                # these are overwritten into one of them.
                 elif type(a) is int or type(a) is float:
                     return a + b
                 # If the first arg is str or other types than above,
@@ -379,7 +385,7 @@ class Stats:
                 # to be the newer value.
                 return a
             ret = {}
-            for data in statistics_data_bypid.values():
+            for data in statistics_data_bymid.values():
                 ret.update(_accum(data, ret))
             return ret
 
@@ -393,31 +399,31 @@ class Stats:
         self.statistics_data = statistics_data
 
         # If the "owner" and "data" arguments in this function are
-        # specified, then the variable of statistics data of each pid
+        # specified, then the variable of statistics data of each module id
         # would be updated.
         errors = []
         if owner and data:
             try:
                 if self.modules[owner].validate_statistics(False, data, errors):
-                    if owner in self.statistics_data_bypid:
-                        if pid in self.statistics_data_bypid[owner]:
-                            self.statistics_data_bypid[owner][pid].update(data)
+                    if owner in self.statistics_data_bymid:
+                        if mid in self.statistics_data_bymid[owner]:
+                            self.statistics_data_bymid[owner][mid].update(data)
                         else:
-                            self.statistics_data_bypid[owner][pid] = data
+                            self.statistics_data_bymid[owner][mid] = data
                     else:
-                        self.statistics_data_bypid[owner] = { pid : data }
+                        self.statistics_data_bymid[owner] = { mid : data }
             except KeyError:
                 errors.append("unknown module name: " + str(owner))
 
         # Just consolidate statistics data of each module without
         # removing that of modules which have been already dead
-        mlist = [ k for k in self.statistics_data_bypid.keys() ]
+        mlist = [ k for k in self.statistics_data_bymid.keys() ]
         for m in mlist:
-            if self.statistics_data_bypid[m]:
+            if self.statistics_data_bymid[m]:
                 if m in self.statistics_data:
                     self.statistics_data[m].update(
                         _accum_bymodule(
-                            self.statistics_data_bypid[m]))
+                            self.statistics_data_bymid[m]))
 
         if errors: return errors
 

src/bin/stats/tests/b10-stats_test.py

@@ -362,35 +362,34 @@ class TestStats(unittest.TestCase):
         self.assertEqual(self.stats.update_statistics_data(owner='Dummy', foo='bar'),
                          ['unknown module name: Dummy'])
 
-    def test_update_statistics_data_withpid(self):
-        # one pid of Auth
+    def test_update_statistics_data_withmid(self):
+        # one id of Auth
         self.stats.update_statistics_data(owner='Auth',
-                                          pid=9999,
+                                          mid="bar@foo",
                                           **{'queries.tcp':1001})
         self.assertTrue('Auth' in self.stats.statistics_data)
         self.assertTrue('queries.tcp' in self.stats.statistics_data['Auth'])
         self.assertEqual(self.stats.statistics_data['Auth']['queries.tcp'], 1001)
-        self.assertTrue('Auth' in self.stats.statistics_data_bypid)
-        self.assertTrue(9999 in self.stats.statistics_data_bypid['Auth'])
-        self.assertTrue('queries.tcp' in self.stats.statistics_data_bypid['Auth'][9999])
-        self.assertEqual(self.stats.statistics_data_bypid['Auth'][9999]['queries.tcp'], 1001)
-        self.assertEqual(self.stats.statistics_data_bypid,
-                         {'Auth': {9999: {'queries.tcp': 1001}}})
-        # check consolidation of statistics data even if there is
-        # non-existent pid of Auth
+        self.assertTrue('Auth' in self.stats.statistics_data_bymid)
+        self.assertTrue('bar@foo' in self.stats.statistics_data_bymid['Auth'])
+        self.assertTrue('queries.tcp' in self.stats.statistics_data_bymid['Auth']['bar@foo'])
+        self.assertEqual(self.stats.statistics_data_bymid['Auth']['bar@foo']['queries.tcp'], 1001)
+        self.assertEqual(self.stats.statistics_data_bymid,
+                         {'Auth': {'bar@foo': {'queries.tcp': 1001}}})
+        # check consolidation of statistics data
         self.stats.update_statistics_data(owner='Auth',
-                                          pid=10000,
+                                          mid="bar2@foo",
                                           **{'queries.tcp':2001})
         self.assertTrue('Auth' in self.stats.statistics_data)
         self.assertTrue('queries.tcp' in self.stats.statistics_data['Auth'])
         self.assertEqual(self.stats.statistics_data['Auth']['queries.tcp'], 3002)
-        self.assertTrue('Auth' in self.stats.statistics_data_bypid)
-        self.assertTrue(9999 in self.stats.statistics_data_bypid['Auth'])
-        self.assertTrue('queries.tcp' in self.stats.statistics_data_bypid['Auth'][9999])
-        self.assertEqual(self.stats.statistics_data_bypid['Auth'][9999]['queries.tcp'], 1001)
-        self.assertEqual(self.stats.statistics_data_bypid,
-                         {'Auth': {9999: {'queries.tcp': 1001},
-                                   10000: {'queries.tcp': 2001}}})
+        self.assertTrue('Auth' in self.stats.statistics_data_bymid)
+        self.assertTrue('bar@foo' in self.stats.statistics_data_bymid['Auth'])
+        self.assertTrue('queries.tcp' in self.stats.statistics_data_bymid['Auth']['bar@foo'])
+        self.assertEqual(self.stats.statistics_data_bymid['Auth']['bar@foo']['queries.tcp'], 1001)
+        self.assertEqual(self.stats.statistics_data_bymid,
+                         {'Auth': {'bar@foo': {'queries.tcp': 1001},
+                                   'bar2@foo': {'queries.tcp': 2001}}})
         # kill running Auth but the statistics data is preserved
         self.assertEqual(self.base.boss.server.pid_list[0][0], 9999)
         killed = self.base.boss.server.pid_list.pop(0)
@@ -400,15 +399,15 @@ class TestStats(unittest.TestCase):
         self.assertTrue('queries.udp' in self.stats.statistics_data['Auth'])
         self.assertEqual(self.stats.statistics_data['Auth']['queries.tcp'], 3002)
         self.assertEqual(self.stats.statistics_data['Auth']['queries.udp'], 0)
-        self.assertTrue('Auth' in self.stats.statistics_data_bypid)
+        self.assertTrue('Auth' in self.stats.statistics_data_bymid)
         # restore statistics data of killed auth
         self.base.boss.server.pid_list = [ killed ] + self.base.boss.server.pid_list[:]
         self.stats.update_statistics_data(owner='Auth',
-                                          pid=9999,
+                                          mid="bar@foo",
                                           **{'queries.tcp':1001})
-        # another pid of Auth
+        # another mid of Auth
         self.stats.update_statistics_data(owner='Auth',
-                                          pid=9998,
+                                          mid="bar3@foo",
                                           **{'queries.tcp':1002,
                                              'queries.udp':1003})
         self.assertTrue('Auth' in self.stats.statistics_data)
@@ -416,15 +415,15 @@ class TestStats(unittest.TestCase):
         self.assertTrue('queries.udp' in self.stats.statistics_data['Auth'])
         self.assertEqual(self.stats.statistics_data['Auth']['queries.tcp'], 4004)
         self.assertEqual(self.stats.statistics_data['Auth']['queries.udp'], 1003)
-        self.assertTrue('Auth' in self.stats.statistics_data_bypid)
-        self.assertTrue(9999 in self.stats.statistics_data_bypid['Auth'])
-        self.assertTrue(9998 in self.stats.statistics_data_bypid['Auth'])
-        self.assertTrue('queries.tcp' in self.stats.statistics_data_bypid['Auth'][9999])
-        self.assertTrue('queries.udp' in self.stats.statistics_data_bypid['Auth'][9998])
-        self.assertTrue('queries.udp' in self.stats.statistics_data_bypid['Auth'][9998])
-        self.assertEqual(self.stats.statistics_data_bypid['Auth'][9999]['queries.tcp'], 1001)
-        self.assertEqual(self.stats.statistics_data_bypid['Auth'][9998]['queries.tcp'], 1002)
-        self.assertEqual(self.stats.statistics_data_bypid['Auth'][9998]['queries.udp'], 1003)
+        self.assertTrue('Auth' in self.stats.statistics_data_bymid)
+        self.assertTrue('bar@foo' in self.stats.statistics_data_bymid['Auth'])
+        self.assertTrue('bar3@foo' in self.stats.statistics_data_bymid['Auth'])
+        self.assertTrue('queries.tcp' in self.stats.statistics_data_bymid['Auth']['bar@foo'])
+        self.assertTrue('queries.udp' in self.stats.statistics_data_bymid['Auth']['bar3@foo'])
+        self.assertTrue('queries.udp' in self.stats.statistics_data_bymid['Auth']['bar3@foo'])
+        self.assertEqual(self.stats.statistics_data_bymid['Auth']['bar@foo']['queries.tcp'], 1001)
+        self.assertEqual(self.stats.statistics_data_bymid['Auth']['bar3@foo']['queries.tcp'], 1002)
+        self.assertEqual(self.stats.statistics_data_bymid['Auth']['bar3@foo']['queries.udp'], 1003)
 
     def test_config(self):
         stats_server = ThreadingServerManager(MyStats)

src/bin/stats/tests/test_utils.py

@@ -426,7 +426,7 @@ class MyStats(stats.Stats):
         try:
             self.start()
         except Exception:
-            pass
+            raise
 
     def shutdown(self):
         self.command_shutdown()