[ovs-dev] [PATCH ovn 0/2] Make ovn-northd recover from NB/SB inconsistencies.

Numan Siddique numans at ovn.org
Thu Apr 30 10:06:08 UTC 2020


On Thu, Apr 30, 2020 at 1:41 PM Numan Siddique <numans at ovn.org> wrote:

>
>
> On Thu, Apr 30, 2020 at 8:43 AM Han Zhou <hzhou at ovn.org> wrote:
>
>> On Wed, Apr 29, 2020 at 4:50 PM Han Zhou <hzhou at ovn.org> wrote:
>> >
>> >
>> >
>> > On Wed, Apr 29, 2020 at 2:45 PM Dumitru Ceara <dceara at redhat.com>
>> wrote:
>> > >
>> > > On 4/29/20 9:57 PM, Han Zhou wrote:
>> > > >
>> > > >
>> > > > On Wed, Apr 29, 2020 at 12:17 PM Numan Siddique <numans at ovn.org
>> > > > <mailto:numans at ovn.org>> wrote:
>> > > >>
>> > > >>
>> > > >>
>> > > >> On Wed, Apr 29, 2020 at 9:57 PM Dumitru Ceara <dceara at redhat.com
>> > > > <mailto:dceara at redhat.com>> wrote:
>> > > >>>
>> > > >>> In some cases, if the NB/SB databases ovn-northd connects to are
>> > > >>> inconsistent, ovn-northd might generate transactions that fail
>> > > >>> continuously due to failed integrity checks on the SB database
>> server.
>> > > >>>
>> > > >>> The first patch of the series addresses inconsistencies due to
>> stale
>> > > >>> Datapath_Binding records in the SB database.
>> > > >>>
>> > > >>> The second patch of the series addresses inconsistencies due to
>> stale
>> > > >>> tunnel_key values in various SB database table records.
>> > > >>>
>> > > >>> Reported-by: Dan Williams <dcbw at redhat.com <mailto:
>> dcbw at redhat.com>>
>> > > >>> Reported-at: https://bugzilla.redhat.com/1828637
>> > > >>> Signed-off-by: Dumitru Ceara <dceara at redhat.com
>> > > > <mailto:dceara at redhat.com>>
>> > > >>>
>> > > >>> Dumitru Ceara (2):
>> > > >>>       ovn-northd: Clear SB records depending on stale datapaths.
>> > > >>>       ovn-northd: Fix tunnel_key allocation for SB records.
>> > > >>
>> > > >>
>> > > >> Hi Dumitru,
>> > > >>
>> > > >> I did some testing in my ovn-fake-multinode setup. These are my
>> > > > observations.
>> > > >>
>> > > >> I created a logical switch sw0 with 4 logical ports. So the next
>> > > > tunnel key should be 5.
>> > > >> I stopped ovn-northd and  created a couple of port_binding entries
>> > > > manually using
>> > > >> "ovn-sbctl create port_binding"  with tunnel keys 5 and 6.
>> > > >> I also created a logical port in sw0. Then I started ovn-northd.
>> > > > ovn-northd deletes the port binding
>> > > >> entries added by me and creates the port_binding entry for the
>> logical
>> > > > port with the tunnel_key=5
>> > > >> in the same transaction.
>> > > >>
>> > > >> I think ovn-northd syncs the south db based on the contents of the
>> > > > north db.
>> > > >>
>> > > >> There's no harm in having your patches. But I'm not really sure if
>> it
>> > > > resolves the issue we have observed.
>> > > >>
>> > > >> Just to brief everyone about the issue we are seeing, we see below
>> > > > logs in ovn-northd.
>> > > >>
>> > > >> *******
>> > > >> 2020-04-16T23:02:33Z|00127|ovsdb_idl|WARN|transaction error:
>> > > > {"details":"Transaction causes multiple rows in \"Port_Binding\"
>> table
>> > > > to have identical values (23eb9016-45f9-4158-be35-77b2713b9a0f and
>> 7)
>> > > > for index on columns \"datapath\" and \"tunnel_key\".  First row,
>> with
>> > > > UUID e4f11a7b-09b6-454f-a125-34cc4b144ef6, had the following index
>> > > > values before the transaction: bdbb436e-f98c-4651-9b80-6e8b95044560
>> and
>> > > > 7.  Second row, with UUID d37cc3f1-8633-440f-b145-8222a0d4723c,
>> existed
>> > > > in the database before this transaction and was not modified by the
>> > > > transaction.","error":"constraint violation"}
>> > > >> ******
>> > > >>
>> > > >> And because of this constraint violation error, ovn-northd cannot
>> > > > further write to the sb db until it is restarted.
>> > > >>
>> > > >> In my opinion this can only happen if ovn-northd doesn't see the
>> port
>> > > > binding row (which is actually present in the DB) in its IDL
>> in-memory db.
>> > > >> I suspect this could have happened when ovn-northd reconnects to
>> the
>> > > > same master or connects to the new master and it doesn't get the
>> proper
>> > > >> updates.
>> > > >>
>> > > >> Maybe in this case, the IDL should request the db contents with txn
>> id
>> > > > =0, so that it receives the complete dump of the db.
>> > > >>
>> > > >> Is it possible that ovn-northd sees a port binding with a tunnel
>> key
>> > > > 'x' and still allocates the same tunnel id 'x' to a new logical
>> port ?
>> > > >> If so, then definitely your patches makes sense.
>> > > >>
>> > > >> @Han - Have you seen this issue in your deployments ? Do you have
>> > > > comments here ?
>> > > >>
>> > > >> Thanks
>> > > >> Numan
>> > > >>
>> > > > I never saw such issue before, but I am not sure if this is possible
>> due
>> > > > to bugs. Currently there is a bug fix under review:
>> > > >
>>
>> https://patchwork.ozlabs.org/project/openvswitch/patch/20200422183842.6303.99600.stgit@dceara.remote.csb/
>> .
>> > > > However, northd doesn't conditionally monitor the rows so I am not
>> sure
>> > > > if this is the root cause of the northd inconsistency issue
>> discussed
>> here.
>> > > >
>> > > > I don't think we should fix in northd (or ovn-controller) to handle
>> the
>> > > > inconsistency of ovsdb. The consistency should be expected from
>> ovsdb
>> > > > and we should fix ovsdb/IDL when there is such kind of bug.
>> Otherwise,
>> > > > there might be too many places to fix and even re-design. My
>> > > > understanding is, if the ovsdb IDL sees a temporarily stale data,
>> the
>> > > > current northd/ovn-controller logic should be able to correct
>> themselves
>> > > > once the data is up-to-date. Moreover, for northd, it is connected
>> to
>> > > > leader-only in clustered mode, which avoids the possibility of
>> seeing
>> > > > staled data in northd (unless there is a bug).
>> > > >
>> > > > To summarize, I think we need to find the root cause of the
>> > > > inconsistency between IDL and server and fix it there, instead of
>> > > > changing ovn-northd to accommodate the inconsistency. (consistency
>> is
>> > > > the biggest advantage of OVSDB, to ease the application
>> implementation).
>> > > >
>> > > > Thanks,
>> > > > Han
>> > >
>> > > Hi Han, Numan,
>> > >
>> > > I might have misused "inconsistency" in this context. What I meant was
>> > > more on the note of "discrepancies between NB and SB databases".
>> > >
>> > > This is a very simple reproducer for the port_binding tunnel_key
>> issue,
>> > > no clustering of NB/SB dbs involved:
>> > >
>> > > # Create two logical switches with one port each.
>> > > $ ovn-nbctl ls-add ls1
>> > > $ ovn-nbctl lsp-add ls1 p1
>> > > $ ovn-nbctl ls-add ls2
>> > > $ ovn-nbctl lsp-add ls2 p2
>> > > $ ovn-nbctl --wait=sb sync
>> > >
>> > > # At this point PB for p1 has tunnel_key=1
>> > > # At this point PB for p2 has tunnel_key=2
>> > >
>> > > # Simulate the SB db going away (could be network
>> > > # issues or crash or some other event).
>> > > $ ovn-ctl stop_sb_ovsdb
>> > >
>> > > # CMS decides to move p2 from ls2 to ls1 and removes
>> > > # ls2 completely.
>> > > $ ovn-nbctl ls-del ls2
>> > > $ ovn-nbctl lsp-add ls1 p2
>> > >
>> > > # Simulate SB DB coming back online.
>> > > $ ovn-ctl start_sb_ovsdb
>> > >
>> > > At this point ovn-northd will try to set the datapath field in PB2 to
>> > > point to datapath_binding corresponding to ls1 but will *not* change
>> > > tunnel_key.
>> > >
>> > > We get:
>> > > 2020-04-29T20:52:41.327Z|00016|ovsdb_idl|WARN|transaction error:
>> > > {"details":"Transaction causes multiple rows in \"Port_Binding\" table
>> > > to have identical values (1b1c4b39-c045-448d-a532-8edbe5544e13 and 1)
>> > > for index on columns \"datapath\" and \"tunnel_key\".  First row, with
>> > > UUID e20219fa-ef67-49a2-81cd-739fa80d2bd4, existed in the database
>> > > before this transaction and was not modified by the transaction.
>> Second
>> > > row, with UUID 50b0e240-8a4d-4e98-8e2f-97c94811d1b1, had the following
>> > > index values before the transaction:
>> > > a9b5959f-2f48-44e7-b6bb-f7148c28e4b5 and 1.","error":"constraint
>> violation"}
>> > >
>> > > And ovn-northd keeps retrying the same transaction at every iteration
>> > > from this point on and fails continuously.
>> > >
>> > > For the stale datapath issue (patch #1 in the series) a similar
>> > > reproducer is:
>> > >
>> > > # Create a logical router with on router port.
>> > > $ ovn-nbctl lr-add lr
>> > > $ ovn-nbctl lrp-add lr p 00:00:00:00:00:01 1.1.1.1/24
>> > >
>> > > # Simulate that a mac binding was created for the router
>> > > # port.
>> > > $ dp=$(ovn-sbctl --bare --columns _uuid list datapath .)
>> > > $ ovn-sbctl create mac_binding logical_port="p" ip="1.1.1.2"
>> datapath="$dp"
>> > > $ ovn-nbctl --wait=sb sync
>> > >
>> > > # Simulate the SB db going away (could be network
>> > > # issues or crash or some other event).
>> > > $ ovn-ctl stop_sb_ovsdb
>> > >
>> > > # CMS decides to delete lr.
>> > > $ ovn-nbctl lr-del lr
>> > >
>> > > # CMS decides to readd lr and router port.
>> > > $ ovn-nbctl lr-add lr
>> > > $ ovn-nbctl lrp-add lr p 00:00:00:00:00:01 1.1.1.1/24
>> > >
>> > > # Simulate SB DB coming back online.
>> > > $ ovn-ctl start_sb_ovsdb
>> > >
>> > > At this point ovn-northd will try to clear the old datapath record
>> from
>> > > SB DB *without* destroying the mac binding record.
>> > >
>> > > We get:
>> > > 2020-04-29T21:41:42.145Z|00013|ovsdb_idl|WARN|transaction error:
>> > > {"details":"cannot delete Datapath_Binding row
>> > > de8d19d6-d67b-499b-8825-12d34ec60946 because of 1 remaining
>> > > reference(s)","error":"referential integrity violation"}
>> > >
>> > > I think both situations above should be addressed by ovn-northd and
>> > > stale datapath/mac_binding/port_binding/etc records should be purged.
>> I
>> > > guess there might be other scenarios that would trigger constraint
>> > > violations too but this is what I found so far.
>> > >
>> > > If you agree, I can send a v2 and add tests for the two simplified
>> > > scenarios I mentioned above.
>> > >
>> > > What do you think?
>>
>
> Thanks Dumitru. for the explanation. It would be great to add these tests
> in v2.
>
> Thanks
> Numan
>
>
>> > >
>> > > Thanks,
>> > > Dumitru
>> > >
>> >
>> > Thanks Dumitru for explaining. Now I understand the problem. So it has
>> nothing to do with OVSDB consistency itself, but just northd'd logic. I
>> don't even need to stop SB to reproduce. Here is how I reproduced it:
>> > $ ovn-nbctl ls-add ls1
>> > $ ovn-nbctl ls-add ls2
>> > $ ovn-nbctl lsp-add ls1 lsp1
>> > $ ovn-nbctl lsp-add ls2 lsp2
>> > $ ovn-nbctl lsp-del ls2 -- lsp-add ls1 lsp2
>>
>> Sorry for the typo. The last command was:
>> $ ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2
>>
>
I applied these 2 patches locally and I ran the below commands, which is
the same as the above
commands shared by Han.

$ovn-nbctl ls-add ls1
$ovn-nbctl ls-add ls2
$ovn-nbctl lsp-add ls1 lsp1
$ovn-nbctl lsp-add ls2 lsp2
$ovs-vsctl add-port br-int p1 -- set Interface p1 external_ids:iface-id=lsp2
$ovn-sbctl list port_binding

$ovn-sbctl list port_binding
_uuid               : bbf2f7e4-b61b-4ce8-adb6-4d17e410b87b
chassis             : ff506354-ac7b-4463-b42d-d89bddf319c7
datapath            : ef316369-0f2c-4246-adbd-8c187bd95e41
...
...
tunnel_key          : 1
type                : ""
virtual_parent      : []

_uuid               : 7cca89fa-55f9-4326-8188-6678838467bb
chassis             : []
datapath            : 21263028-a511-457a-824b-39a1219084c8
...
logical_port        : lsp1
...
tunnel_key          : 1
type                : ""
virtual_parent      : []

$ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2

$ovn-sbctl list port_binding
_uuid               : bbf2f7e4-b61b-4ce8-adb6-4d17e410b87b
chassis             : ff506354-ac7b-4463-b42d-d89bddf319c7
datapath            : 21263028-a511-457a-824b-39a1219084c8
...
logical_port        : lsp2
...
tunnel_key          : 1
type                : ""
virtual_parent      : []

_uuid               : 7cca89fa-55f9-4326-8188-6678838467bb
chassis             : []
datapath            : 21263028-a511-457a-824b-39a1219084c8
...
logical_port        : lsp1
...
tunnel_key          : 2
type                : ""
virtual_parent      : []


I notice that the same port_binding record for lsp2 is being reused.
Is that intentional ?

Ideally the old port binding record lsp2 should get deleted and
new one should get created.

I found another issue with the below commands (tested in sandbox env)

$ovn-nbctl ls-add ls1
$ovn-nbctl ls-add ls2
$ovn-nbctl lsp-add ls1 lsp1
$ovn-nbctl lsp-add ls2 lsp2
$ovn-nbctl lsp-set-type lsp2 external
$ovn-nbctl ha-chassis-group-add chg1
$ovn-nbctl ha-chassis-group-add-chassis chg1 chassis-1 30
$ovn-nbctl set logical_switch_port lsp2 ha_chassis_group=<chg1_uuid>
$ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2 -> This fails with the below
logs in ovn-northd

*******
2020-04-30T09:59:48.319Z|00007|ovsdb_idl|WARN|transaction error:
{"details":"cannot delete HA_Chassis_Group row
6e0c88d7-20f6-473a-bd0a-9eea60b639e6 because of 1 remaining
reference(s)","error":"referential integrity violation"}
*******

I think it's better if the stale port binding entry is deleted instead of
reusing it. What  do you think ?

Thanks
Numan




>> >
>> > 2020-04-29T23:46:17.675Z|00007|ovsdb_idl|WARN|transaction error:
>> {"details":"Transaction causes multiple rows in \"Port_Binding\" table to
>> have identical values (be595a3b-3904-4229-9ba2-884b27a86b75 and 1) for
>> index on columns \"datapath\" and \"tunnel_key\".  First row, with UUID
>> d4cc6ec5-4817-47c9-aa83-9985d3b7b452, existed in the database before this
>> transaction and was not modified by the transaction.  Second row, with
>> UUID
>> b874ab93-d97a-4583-8ac3-c353a40b180d, had the following index values
>> before
>> the transaction: 6940ad91-83c5-4fe9-bab5-4fbec6714b0d and
>> 1.","error":"constraint violation"}
>> >
>> > I will take a closer look at the fix.
>> >
>> > Thanks,
>> > Han
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>


More information about the dev mailing list