Re: [ovs-dev] [PATCH v1] Monitor Database table to manage lifecycle of IDL client.
Thanks for taking a look. I will send a revised patch that addresses both issues you pointed to, in particular backward compatibility with _Server-less ovsdbs. Thanks, Ted On 12/27/18, 9:23 AM, "Ben Pfaff" wrote: On Tue, Dec 18, 2018 at 12:18:06AM +, Ted Elhourani wrote: > The Python IDL implementation supports ovsdb cluster connections. > This patch is a follow up to commit 31e434fc98, it adds the option of > connecting to the leader in the Raft-based cluster. It mimics the > exisiting C IDL support for clusters introduced in commit 1b1d2e6daa. > > The server schema is first requested, then a monitor of the Database table > in the _Server Database. __check_server_db is called to verify the > eligibility of the server. Unlike the C IDL, if an attempt to obtain a > monitor of the Server database fails this implementation does not proceed > to monitor and transact with the database. The reason is connections to > standalone databases should not be permitted when the intention is to > connect to a valid cluster node. The change_seqno is not incremented in the > case of Database table updates. > > The run method of Session (jsonrpc) is modified such that the session is > closed when a stream connection fails, if more than one remote exists. > Otherwise the session will re-attempt to establish a stream to the same > cluster node. Method pick_remote must be called before opening a stream > in __connect (in Session), otherwise a force_reconnect causes the Session > to re-attempt a connection to the same server. pick_remote is no longer > necessary in method run of Session. > > Signed-off-by: Ted Elhourani I'm pretty sure that this breaks backward compatibility with any ovsdb-server not new enough to support the _Server table. The commit message above conflates that with whether the database is clustered. I think your intention is to reject non-clustered databases, but it doesn't do that; instead, it rejects database servers too old to have a _Server table and accepts standalone databases served up by new-enough servers. It's one thing to avoid connecting to standalone databases when a cluster is expected, but this goes beyond that to drop support for all older database servers. I don't think that's reasonable behavior. I noticed that the following looks wrong. EAGAIN indicates that the connection is not complete yet. It should not be treated as a connection failure (unless a timeout has occurred) even if there is more than one remote: > @@ -516,9 +516,9 @@ class Session(object): > self.reconnect.connected(ovs.timeval.msec()) > self.rpc = Connection(self.stream) > self.stream = None > -elif error != errno.EAGAIN: > +elif (error != errno.EAGAIN or > + self.get_num_of_remotes() > 1): > self.reconnect.connect_failed(ovs.timeval.msec(), error) > -self.pick_remote() > self.stream.close() > self.stream = None Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Monitor Database table to manage lifecycle of IDL client.
On Tue, Dec 18, 2018 at 12:18:06AM +, Ted Elhourani wrote: > The Python IDL implementation supports ovsdb cluster connections. > This patch is a follow up to commit 31e434fc98, it adds the option of > connecting to the leader in the Raft-based cluster. It mimics the > exisiting C IDL support for clusters introduced in commit 1b1d2e6daa. > > The server schema is first requested, then a monitor of the Database table > in the _Server Database. __check_server_db is called to verify the > eligibility of the server. Unlike the C IDL, if an attempt to obtain a > monitor of the Server database fails this implementation does not proceed > to monitor and transact with the database. The reason is connections to > standalone databases should not be permitted when the intention is to > connect to a valid cluster node. The change_seqno is not incremented in the > case of Database table updates. > > The run method of Session (jsonrpc) is modified such that the session is > closed when a stream connection fails, if more than one remote exists. > Otherwise the session will re-attempt to establish a stream to the same > cluster node. Method pick_remote must be called before opening a stream > in __connect (in Session), otherwise a force_reconnect causes the Session > to re-attempt a connection to the same server. pick_remote is no longer > necessary in method run of Session. > > Signed-off-by: Ted Elhourani I'm pretty sure that this breaks backward compatibility with any ovsdb-server not new enough to support the _Server table. The commit message above conflates that with whether the database is clustered. I think your intention is to reject non-clustered databases, but it doesn't do that; instead, it rejects database servers too old to have a _Server table and accepts standalone databases served up by new-enough servers. It's one thing to avoid connecting to standalone databases when a cluster is expected, but this goes beyond that to drop support for all older database servers. I don't think that's reasonable behavior. I noticed that the following looks wrong. EAGAIN indicates that the connection is not complete yet. It should not be treated as a connection failure (unless a timeout has occurred) even if there is more than one remote: > @@ -516,9 +516,9 @@ class Session(object): > self.reconnect.connected(ovs.timeval.msec()) > self.rpc = Connection(self.stream) > self.stream = None > -elif error != errno.EAGAIN: > +elif (error != errno.EAGAIN or > + self.get_num_of_remotes() > 1): > self.reconnect.connect_failed(ovs.timeval.msec(), error) > -self.pick_remote() > self.stream.close() > self.stream = None Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v1] Monitor Database table to manage lifecycle of IDL client.
The Python IDL implementation supports ovsdb cluster connections. This patch is a follow up to commit 31e434fc98, it adds the option of connecting to the leader in the Raft-based cluster. It mimics the exisiting C IDL support for clusters introduced in commit 1b1d2e6daa. The server schema is first requested, then a monitor of the Database table in the _Server Database. __check_server_db is called to verify the eligibility of the server. Unlike the C IDL, if an attempt to obtain a monitor of the Server database fails this implementation does not proceed to monitor and transact with the database. The reason is connections to standalone databases should not be permitted when the intention is to connect to a valid cluster node. The change_seqno is not incremented in the case of Database table updates. The run method of Session (jsonrpc) is modified such that the session is closed when a stream connection fails, if more than one remote exists. Otherwise the session will re-attempt to establish a stream to the same cluster node. Method pick_remote must be called before opening a stream in __connect (in Session), otherwise a force_reconnect causes the Session to re-attempt a connection to the same server. pick_remote is no longer necessary in method run of Session. Signed-off-by: Ted Elhourani --- python/ovs/db/idl.py| 189 python/ovs/jsonrpc.py | 10 +-- python/ovs/reconnect.py | 3 + tests/ovsdb-idl.at | 62 4 files changed, 214 insertions(+), 50 deletions(-) diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index 250e897..f5edf5e 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -38,6 +38,7 @@ ROW_DELETE = "delete" OVSDB_UPDATE = 0 OVSDB_UPDATE2 = 1 +CLUSTERED = "clustered" class Idl(object): """Open vSwitch Database Interface Definition Language (OVSDB IDL). @@ -92,10 +93,12 @@ class Idl(object): """ IDL_S_INITIAL = 0 -IDL_S_MONITOR_REQUESTED = 1 -IDL_S_MONITOR_COND_REQUESTED = 2 +IDL_S_SERVER_SCHEMA_REQUESTED = 1 +IDL_S_SERVER_MONITOR_REQUESTED = 2 +IDL_S_DATA_MONITOR_REQUESTED = 3 +IDL_S_DATA_MONITOR_COND_REQUESTED = 4 -def __init__(self, remote, schema_helper, probe_interval=None): +def __init__(self, remote, schema_helper, leader_only=True, probe_interval=None): """Creates and returns a connection to the database named 'db_name' on 'remote', which should be in a form acceptable to ovs.jsonrpc.session.open(). The connection will maintain an in-memory @@ -119,6 +122,9 @@ class Idl(object): The IDL uses and modifies 'schema' directly. +If 'leader_only' is set to True (default value) the IDL will only monitor +and transact with the leader of the cluster. + If "probe_interval" is zero it disables the connection keepalive feature. If non-zero the value will be forced to at least 1000 milliseconds. If None it will just use the default value in OVS. @@ -137,6 +143,20 @@ class Idl(object): self._last_seqno = None self.change_seqno = 0 self.uuid = uuid.uuid1() + +# Server monitor. +self._server_schema_request_id = None +self._server_monitor_request_id = None +self._db_change_aware_request_id = None +self._server_db_name = '_Server' +self._server_db_table = 'Database' +self.server_tables = None +self._server_db = None +self.server_monitor_uuid = uuid.uuid1() +self.leader_only = leader_only +self.cluster_id = None +self._min_index = 0 + self.state = self.IDL_S_INITIAL # Database locking. @@ -172,6 +192,10 @@ class Idl(object): remotes.append(r) return remotes +def set_cluster_id(self, cluster_id): +"""Set the id of the cluster that this idl must connect to.""" +self.cluster_id = str(cluster_id) + def index_create(self, table, name): """Create a named multi-column index on a table""" return self.tables[table].rows.index_create(name) @@ -222,7 +246,7 @@ class Idl(object): if seqno != self._last_seqno: self._last_seqno = seqno self.__txn_abort_all() -self.__send_monitor_request() +self.__send_server_schema_request() if self.lock_name: self.__send_lock_request() break @@ -239,7 +263,14 @@ class Idl(object): and msg.method == "update" and len(msg.params) == 2): # Database contents changed. -self.__parse_update(msg.params[1], OVSDB_UPDATE) +if msg.params[0] == str(self.server_monitor_uuid): +self.__parse_update(msg.params[1], OVSDB_UPDATE, tables=self.server_tables) +self.change_seqno = initial_change_seqno +
