Hi Ilya, > On Tuesday, September 7, 2021 7:56 PM, Ilya Maximets > [mailto:i.maxim...@ovn.org] wrote: > I had a patch somewhere. I guess, I need to revise it and send.
Can you info me where to get your patch, I would like to test it and reply you the test result. Best Regards! Gang Chen > -----Original Message----- > From: Ilya Maximets [mailto:i.maxim...@ovn.org] > Sent: Tuesday, September 7, 2021 7:56 PM > To: Chengang (Russell Lab) <chengan...@huawei.com>; Han Zhou > <hz...@ovn.org>; Ilya Maximets <i.maxim...@ovn.org> > Cc: d...@openvswitch.org; wangyunjian <wangyunj...@huawei.com>; > dingxiaoxiong <dingxiaoxi...@huawei.com>; Dumitru Ceara > <dce...@redhat.com> > Subject: Re: [ovs-dev] [PATCH] ovsdb-idl: Send "set_db_change_aware" before > "monitor_cond_since". > > On 9/6/21 3:51 PM, Chengang (Russell Lab) wrote: > > Hi Han and Ilya, > > > > This patch fix the issue: IDL client closes the connection after got > > the response to the monitor_cond_since request, without wait for the > > reponse to the set_db_change_aware request. Warning message will be > > add to ovsdb-server.log > > > > It makes user confused the issue will occurred even with the command > "ovs-vsctl show" or ovs-vsctl other commands. > > > > Is there any other ideas on this issue? > > The root cause of a problem is that set_db_change_aware request is hacked > into the IDL state machine and IDL doesn't wait for reply and doesn't handle > the reply in any way. Moving this hack around the IDl code doesn't seem to be > a good solution as it interferes with the rest of the state machine which we > definitely don't want to accidentally break. > > The correct way to eliminate these benign warnings, as we discussed > previously, is to create a new set_db_change_aware notification (not a > request), so it will not require sending reply from a server. This will fix > the > problem and will also make IDL code logically correct. The downside of this > is > that if new clients will connect to old servers, they will not be able to use > the > feature if notification is used. So, there should be some transition period > until > we can enable use of notifications by default. > > I had a patch somewhere. I guess, I need to revise it and send. > > > If this patch can fix it, can we back port this patch to 2.13.0 as it has > > the same > issue? > > The issue exists starting from 2.9, however, the correct solution (to use > notifications) doesn't seem backportable. > > Best regards, Ilya Maximets. > > > > > Best Regards! > > Gang Chen > > > >> -----Original Message----- > >> From: dev [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of > >> Dumitru Ceara > >> Sent: Friday, July 10, 2020 7:33 PM > >> To: d...@openvswitch.org > >> Cc: Han Zhou <hz...@ovn.org>; Ilya Maximets <i.maxim...@ovn.org> > >> Subject: [ovs-dev] [PATCH] ovsdb-idl: Send "set_db_change_aware" > >> before "monitor_cond_since". > >> > >> For short lived IDL clients (e.g., ovn-sbctl) if the client sends > >> monitor_cond_since before set_db_change_aware, the client might close > >> the DB connection immediately after it received the reply for > >> monitor_cond_since and before the server has a chance to reply to > set_db_change_aware. > >> > >> E.g., from the logs of the ovsdb-server: > >> 2020-07-10T09:29:52.649Z|04479|jsonrpc|DBG|unix#72: received request, > >> method="monitor_cond_since", params=["OVN_Southbound", > >> ["monid","OVN_Southbound"],{"SB_Global":[{"columns":["options"]}]}, > >> "00000000-0000-0000-0000-000000000000"], id=2 > >> 2020-07-10T09:29:52.649Z|04480|jsonrpc|DBG|unix#72: send reply, > >> result=[false,"00000000-0000-0000-0000-000000000000", > >> {"SB_Global":{"6ad26b48-a742-4fe1-8671-3975e2146ce6":{"initial": > >> {"options":["map",[["mac_prefix","be:85:cb"],["svc_monitor_mac", > >> "52:58:b5:19:8c:40"]]]}}}}], id=2 > >> 2020-07-10T09:29:52.649Z|04482|jsonrpc|DBG|unix#72: received request, > >> method="set_db_change_aware", params=[true], id=3 > >> > >> <<< IDL client closes the connection here because it already got the > >> response to the monitor_cond_since request. > >> > >> 2020-07-10T09:29:59.023Z|04483|jsonrpc|DBG|unix#72: send reply, > >> result={}, > >> id=3 > >> 2020-07-10T09:29:59.023Z|04484|stream_fd|DBG|send: Broken pipe > >> 2020-07-10T09:29:59.023Z|04485|jsonrpc|WARN|unix#72: send error: > >> Broken pipe > >> > >> While this is not a critical issue, it can be easily mitigated by > >> changing the IDL client to always send "set_db_change_aware" before > "monitor_cond_since". > >> This way we ensure that a well behaving IDL client doesn't close the > >> connection too early, avoiding the error logs in ovsdb-server. > >> > >> This patch moves the code to send monitor_cond_since(data) from > >> function > >> ovsdb_idl_check_server_db() to ovsdb_idl_process_response() as we can > >> transition to IDL_S_DATA_MONITOR_COND_SINCE_REQUESTED only upon > >> receiving a reply for monitor_cond(server). > >> > >> CC: Ben Pfaff <b...@ovn.org> > >> CC: Han Zhou <hz...@ovn.org> > >> CC: Ilya Maximets <i.maxim...@ovn.org> > >> Reported-by: Girish Moodalbail <gmoodalb...@gmail.com> > >> Reported-at: > >> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-July/050343.h > >> tml > >> Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for > >> clustered > >> databases.") > >> Signed-off-by: Dumitru Ceara <dce...@redhat.com> > >> --- > >> lib/ovsdb-idl.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index ef3b97b..c6427f5 > >> 100644 > >> --- a/lib/ovsdb-idl.c > >> +++ b/lib/ovsdb-idl.c > >> @@ -770,6 +770,10 @@ ovsdb_idl_process_response(struct ovsdb_idl > >> *idl, struct jsonrpc_msg *msg) > >> > >> OVSDB_IDL_MM_MONITOR_COND); > >> if (ovsdb_idl_check_server_db(idl)) { > >> ovsdb_idl_send_db_change_aware(idl); > >> + ovsdb_idl_send_monitor_request( > >> + idl, &idl->data, > >> OVSDB_IDL_MM_MONITOR_COND_SINCE); > >> + ovsdb_idl_transition( > >> + idl, > >> IDL_S_DATA_MONITOR_COND_SINCE_REQUESTED); > >> } > >> } else { > >> ovsdb_idl_send_schema_request(idl, &idl->data); @@ > >> -2057,9 > >> +2061,6 @@ ovsdb_idl_check_server_db(struct ovsdb_idl *idl) > >> if (idl->state == IDL_S_SERVER_MONITOR_COND_REQUESTED) { > >> json_destroy(idl->data.schema); > >> idl->data.schema = json_from_string(database->schema); > >> - ovsdb_idl_send_monitor_request(idl, &idl->data, > >> - > >> OVSDB_IDL_MM_MONITOR_COND_SINCE); > >> - ovsdb_idl_transition(idl, > >> IDL_S_DATA_MONITOR_COND_SINCE_REQUESTED); > >> } > >> return true; > >> } > >> -- > >> 1.8.3.1 > >> > >> _______________________________________________ > >> dev mailing list > >> d...@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev