From: Ondrej Lichtner olichtne@redhat.com
This commit reimplements the ConnectionHandler class while kkeping the same interface. The main point was to store file handles in a list instead of a dictionary which can have name conflicts. The old dictionary is still used but only to map a connection name to the connection. This allows us to introduce more mappings in derived classes and keep the connection checking code from the main class.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/ConnectionHandler.py | 43 +++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/lnst/Common/ConnectionHandler.py b/lnst/Common/ConnectionHandler.py index e8a6976..36349dc 100644 --- a/lnst/Common/ConnectionHandler.py +++ b/lnst/Common/ConnectionHandler.py @@ -67,18 +67,22 @@ def recv_data(s):
class ConnectionHandler(object): def __init__(self): - self._connections = {} + self._connections = [] + self._connection_mapping = {}
def check_connections(self): - return self.check_connections_by_id(self._connections.keys()) + return self._check_connections(self._connections)
def check_connections_by_id(self, connection_ids): connections = [] for con_id in connection_ids: - connections.append(self._connections[con_id]) + connections.append(self._connection_mapping[con_id]) + return self._check_connections(connections) + + def _check_connections(self, connections): requests = [] try: - rl, wl, xl = select.select(connections, [], [], 0) + rl, wl, xl = select.select(connections, [], []) except select.error: return [] for f in rl: @@ -112,27 +116,34 @@ class ConnectionHandler(object): return requests
def get_connection(self, id): - if id in self._connections: - return self._connections[id] + if id in self._connection_mapping: + return self._connection_mapping[id] else: return None
def get_connection_id(self, connection): - for id in self._connections: - if self._connections[id] == connection: + for id in self._connection_mapping: + if self._connection_mapping[id] == connection: return id return None
def add_connection(self, id, connection): - if id not in self._connections: - self._connections[id] = connection + if id not in self._connection_mapping: + self._connections.append(connection) + self._connection_mapping[id] = connection
def remove_connection(self, connection): - d = {} - for key, value in self._connections.iteritems(): - if value != connection: - d[key] = value - self._connections = d + if connection in self._connections: + id = self.get_connection_id(connection) + self._connections.remove(connection) + del self._connection_mapping[id] + + def remove_connection_by_id(self, id): + if id in self._connection_mapping: + connection = self._connection_mapping[id] + self._connections.remove(connection) + del self._connection_mapping[id]
def clear_connections(self): - self._connections = {} + self._connections = [] + self._connection_mapping = {}
From: Ondrej Lichtner olichtne@redhat.com
This patch updates both the MessageDispatcher and the MessageDispatcherLite classes to use the _connection_mapping attribute of the parent class.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Controller/MessageDispatcherLite.py | 4 ++-- lnst/Controller/NetTestController.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lnst/Controller/MessageDispatcherLite.py b/lnst/Controller/MessageDispatcherLite.py index 70a006a..8e26d17 100644 --- a/lnst/Controller/MessageDispatcherLite.py +++ b/lnst/Controller/MessageDispatcherLite.py @@ -31,11 +31,11 @@ class MessageDispatcherLite(ConnectionHandler): def wait_for_result(self, machine_id): wait = True while wait: - connected_slaves = self._connections.keys() + connected_slaves = self._connection_mapping.keys()
messages = self.check_connections()
- remaining_slaves = self._connections.keys() + remaining_slaves = self._connection_mapping.keys()
for msg in messages: if msg[1]["type"] == "result" and msg[0] == 1: diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py index da7df41..bdf1bd3 100644 --- a/lnst/Controller/NetTestController.py +++ b/lnst/Controller/NetTestController.py @@ -836,11 +836,11 @@ class MessageDispatcher(ConnectionHandler): def wait_for_result(self, machine_id): wait = True while wait: - connected_slaves = self._connections.keys() + connected_slaves = self._connection_mapping.keys()
messages = self.check_connections()
- remaining_slaves = self._connections.keys() + remaining_slaves = self._connection_mapping.keys()
for msg in messages: if msg[1]["type"] == "result" and msg[0] == machine_id:
From: Ondrej Lichtner olichtne@redhat.com
This patch makes the class ServerHandler a class derived from the ConnectionHandler. In addition to the standard connection_mapping it adds a new _netns_con_mapping that maps network namespaces to Pipe objects.
This solves the problems of the previous implementation where we would have two ConnectionHandlers controlled by the ServerHandler which caused either 100% CPU consumption or slowdowns because of select timeouts.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Slave/NetTestSlave.py | 59 ++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 31 deletions(-)
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index bd023cb..c63c719 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -608,10 +608,10 @@ class SlaveMethods: device.set_netns(None) return True
-class ServerHandler(object): +class ServerHandler(ConnectionHandler): def __init__(self, addr): - self._connection_handler = ConnectionHandler() - self._netns_con_handler = ConnectionHandler() + super(ServerHandler, self).__init__() + self._netns_con_mapping = {} try: self._s_socket = socket.socket() self._s_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) @@ -621,16 +621,15 @@ class ServerHandler(object): logging.error(e[1]) exit(1)
- self._c_socket = None self._netns = None + self._c_socket = None
def accept_connection(self): self._c_socket, addr = self._s_socket.accept() self._c_socket = (self._c_socket, addr[0]) logging.info("Recieved connection from %s" % self._c_socket[1])
- self._connection_handler.add_connection(self._c_socket[1], - self._c_socket[0]) + self.add_connection(self._c_socket[1], self._c_socket[0]) return self._c_socket
def get_ctl_sock(self): @@ -644,8 +643,7 @@ class ServerHandler(object): self._c_socket.close() self._c_socket = None self._c_socket = sock - self._connection_handler.add_connection(self._c_socket[1], - self._c_socket[0]) + self.add_connection(self._c_socket[1], self._c_socket[0])
def close_s_sock(self): self._s_socket.close() @@ -653,12 +651,11 @@ class ServerHandler(object):
def close_c_sock(self): self._c_socket[0].close() - self._connection_handler.remove_connection(self._c_socket[0]) + self.remove_connection(self._c_socket[0]) self._c_socket = None
def get_messages(self): - messages = self._connection_handler.check_connections() - messages += self._netns_con_handler.check_connections() + messages = self.check_connections()
#push ctl messages to the end of message queue, this ensures that #update messages are handled first @@ -672,18 +669,19 @@ class ServerHandler(object): messages = non_ctl_msgs + ctl_msgs
addr = self._c_socket[1] - if self._connection_handler.get_connection(addr) == None: + if self.get_connection(addr) == None: logging.info("Lost controller connection.") self._c_socket = None return messages
def get_messages_from_con(self, con_id): - if self._connection_handler.get_connection(con_id) != None: - return self._connection_handler.check_connections_by_id([con_id]) - elif self._netns_con_handler.get_connection(con_id) != None: - return self._netns_con_handler.check_connections_by_id([con_id]) + if con_id in self._connection_mapping: + connection = self._connection_mapping[con_id] + elif con_id in self._netns_con_mapping: + connection = self._netns_con_mapping[con_id] else: raise Exception("Unknown connection id '%s'." % con_id) + return self._check_connections([connection])
def send_data_to_ctl(self, data): if self._c_socket != None: @@ -696,39 +694,38 @@ class ServerHandler(object): return False
def send_data_to_netns(self, netns, data): - netns_con = self._netns_con_handler.get_connection(netns) - if netns_con == None: + if netns not in self._netns_con_mapping: raise Exception("No such namespace!") else: + netns_con = self._netns_con_mapping[netns] return send_data(netns_con, data)
- def add_connection(self, id, connection): - self._connection_handler.add_connection(id, connection) - - def remove_connection(self, key): - connection = self._connection_handler.get_connection(key) - self._connection_handler.remove_connection(connection) - def clear_connections(self): - self._connection_handler.clear_connections() + super(ServerHandler, self).clear_connections() + self._netns_con_mapping = {}
def update_connections(self, connections): for key, connection in connections.iteritems(): - self.remove_connection(key) + self.remove_connection_by_id(key) self.add_connection(key, connection)
def set_netns(self, netns): self._netns = netns
def add_netns(self, netns, connection): - self._netns_con_handler.add_connection(netns, connection) + self._connections.append(connection) + self._netns_con_mapping[netns] = connection
def del_netns(self, netns): - connection = self._netns_con_handler.get_connection(netns) - self._netns_con_handler.remove_connection(connection) + if netns in self._netns_con_mapping: + connection = self._netns_con_mapping[netns] + self._connections.remove(connection) + del self._netns_con_mapping[netns]
def clear_netns_connections(self): - self._netns_con_handler.clear_connections() + for netns, con in self._netns_con_mapping: + self._connections.remove(con) + self._netns_con_mapping = {}
class NetTestSlave: def __init__(self, log_ctl, port = DefaultRPCPort):
Tue, Sep 30, 2014 at 01:25:31PM CEST, olichtne@redhat.com wrote:
From: Ondrej Lichtner olichtne@redhat.com
This commit reimplements the ConnectionHandler class while kkeping the same interface. The main point was to store file handles in a list instead of a dictionary which can have name conflicts. The old dictionary is still used but only to map a connection name to the connection. This allows us to introduce more mappings in derived classes and keep the connection checking code from the main class.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com
all applied, thanks!
lnst-developers@lists.fedorahosted.org
-
Jiri Pirko -
Ondrej Lichtner