client: for read accesses, pick a random good node, connected or not

The issue was that at startup, or after nodes are back, the previous code
prevented full load balancing until some data are written.

It was like this to limit the number of connections, which does not matter
anymore (see commit 77132157).

neo/client/pool.py

@@ -56,25 +56,28 @@ class ConnectionPool(object):
             logging.info('%r not ready', node)
         else:
             logging.info('Connected %r', node)
+            # Make sure this node will be considered for the next reads
+            # even if there was a previous recent failure.
+            self.node_failure_dict.pop(node.getUUID(), None)
             return conn
         self.node_failure_dict[node.getUUID()] = time.time() + MAX_FAILURE_AGE
 
     def getCellSortKey(self, cell, random=random.random):
-        # The use of 'random' suffles cells to randomise node to access.
-        uuid = cell.getUUID()
-        # First, prefer a connected node.
-        if uuid in self.connection_dict:
-            return random()
-        # Then one that didn't fail recently.
-        failure = self.node_failure_dict.get(uuid)
+        # Prefer a node that didn't fail recently.
+        failure = self.node_failure_dict.get(cell.getUUID())
         if failure:
             if time.time() < failure:
-                # At last, order by date of connection failure.
+                # Or order by date of connection failure.
                 return failure
             # Do not use 'del' statement: we didn't lock, so another
             # thread might have removed uuid from node_failure_dict.
-            self.node_failure_dict.pop(uuid, None)
-        return 1 + random()
+            self.node_failure_dict.pop(cell.getUUID(), None)
+        # A random one, connected or not, is a trivial and quite efficient way
+        # to distribute the load evenly. On write accesses, a client connects
+        # to all nodes of touched cells, but before that, or if a client is
+        # specialized to only do read-only accesses, it should not limit
+        # itself to only use the first connected nodes.
+        return random()
 
     def getConnForNode(self, node):
         """Return a locked connection object to a given node

neo/tests/threaded/test.py

@@ -480,17 +480,18 @@ class Test(NEOThreadedTest):
     def test_notifyNodeInformation(self, cluster):
         # translated from MasterNotificationsHandlerTests
         # (neo.tests.client.testMasterHandler)
+        good = [1, 0].pop
         if 1:
             cluster.db # open DB
             s0, s1 = cluster.client.nm.getStorageList()
             conn = s0.getConnection()
             self.assertFalse(conn.isClosed())
             getCellSortKey = cluster.client.cp.getCellSortKey
-            self.assertEqual(getCellSortKey(s0, int), 0)
+            self.assertEqual(getCellSortKey(s0, good), 0)
             cluster.neoctl.dropNode(s0.getUUID())
             self.assertEqual([s1], cluster.client.nm.getStorageList())
             self.assertTrue(conn.isClosed())
-            self.assertEqual(getCellSortKey(s0, int), 1)
+            self.assertEqual(getCellSortKey(s0, good), 1)
             # XXX: the test originally checked that 'unregister' method
             #      was called (even if it's useless in this case),
             #      but we would need an API to do that easily.