<div dir="ltr"><div>Hello Balazs:</div><div><br></div><div>You are right: a DB migration is not necessary but a DB sanitization. I did something similar in <a href="https://review.opendev.org/c/openstack/neutron/+/789831">https://review.opendev.org/c/openstack/neutron/+/789831</a>. This patch includes the script to, in one shot, modify the whole DB and an upgrade check to test it.<br></div><div><br></div><div>About the code, if something like the script provided and the upgrade check is included, that will ensure the DB is correctly migrated in Y release. That means the code supporting both formats could be removed in Z.</div><div><br></div><div>Regards.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 22, 2021 at 6:54 PM Balazs Gibizer <balazs.gibizer@est.tech> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On Wed, Sep 22 2021 at 05:09:17 PM +0200, Rodolfo Alonso Hernandez <br>
<<a href="mailto:ralonsoh@redhat.com" target="_blank">ralonsoh@redhat.com</a>> wrote:<br>
> Hello Balazs:<br>
> <br>
> About the group_id, I see this is built using the port_id and the <br>
> qos_rules. We have all of this in the DB and we can build it <br>
> statically (I think so, maybe I'm very optimistic).<br>
<br>
Yes, that is probably doable. But I let Przemek work out the details in <br>
the patch.<br>
> <br>
> About the code, that was something I was thinking about after sending <br>
> the last mail. For at least two releases, we need to support both RP <br>
> formats in the DB. If we read only the UUID (old format), then we <br>
> should convert it and store it in the new format.<br>
> <br>
> About the migration, we don't support contract migrations anymore. <br>
> But this is not true as we have done some migrations that have added <br>
> new restrictions in the DB schema. In any case, this could be done as <br>
> an expansion migration. If the code is in place, I don't see any <br>
> problem of doing this migration with the server running. Each <br>
> "ml2_port_bindings" register will be updated atomically, while the <br>
> Neutron server will be able to handle both versions.<br>
> <br>
<br>
If I understand correctly what you described is an online data <br>
migration where<br>
1) neutron does not even need an expand migration as no new field is <br>
needed in the database as we use the old field both for the old and the <br>
new data<br>
2) neutron server converts between data formats when the data is read<br>
3) neutron can drop the conversion code only after every register is <br>
upgraded this way. As there could be ports that are not touched between <br>
upgrades we cannot simply say that we are done with the migration after <br>
waiting a release cycle. I think we have to add an upgrade check in <br>
Yoga that warns if there are still ports with the old format. And also <br>
neutron needs to provide a tool for the deployer to trigger the <br>
conversion of those remaining port before the upgrade to X.<br>
<br>
Do I understand your suggestion correct? Do you agree with the above <br>
solution proposal?<br>
<br>
Cheers,<br>
gibi<br>
<br>
> Regards.<br>
> <br>
> <br>
> On Wed, Sep 22, 2021 at 3:44 PM Balazs Gibizer <br>
> <balazs.gibizer@est.tech> wrote:<br>
>> <br>
>> <br>
>>  On Tue, Sep 21 2021 at 06:30:46 PM +0200, Rodolfo Alonso Hernandez<br>
>>  <<a href="mailto:ralonsoh@redhat.com" target="_blank">ralonsoh@redhat.com</a>> wrote:<br>
>>  > Hello Balazs:<br>
>>  ><br>
>>  > Sorry for the late reply, I was on PTO.<br>
>>  ><br>
>>  > If I'm not wrong, now port['binding:profile']['allocation'] is a <br>
>> UUID<br>
>>  > and you need it to be a list of UUIDs. Am I correct?<br>
>> <br>
>>  It is a bit more complicated than that. The old value is a single RP<br>
>>  UUID. the new value is a dict where the key is the group_id and the<br>
>>  value is the RP UUID fulfilling that group. So the transformation <br>
>> needs<br>
>>  to access to the group_id.<br>
>>  The group_id is a stable UUID generated by neutron server as part of<br>
>>  the port.resource_request value, but it is not persisted.<br>
>> <br>
>>  ><br>
>>  > To make this change in the DB you should use the Alembic <br>
>> migrations,<br>
>>  > as you said. That should ensure all registers are translated. We<br>
>>  > should also include a sanity check to ensure the DB migration was<br>
>>  > done correctly.<br>
>> <br>
>>  I'm not 100% sure but I think such data migration can only be done <br>
>> in<br>
>>  the contract part as it needs to be done while the neutron server is<br>
>>  down as the old code can only use the old data format while the new<br>
>>  code can only use the new format. Is it OK to introduce a new <br>
>> contract<br>
>>  migration in Yoga in neutron?<br>
>> <br>
>>  Cheers,<br>
>>  gibi<br>
>> <br>
>> <br>
>>  ><br>
>>  > Is that what you needed? Don't hesitate to ping me in IRC if <br>
>> needed.<br>
>>  ><br>
>>  > Regards.<br>
>>  ><br>
>>  > On Fri, Sep 10, 2021 at 6:06 PM Balazs Gibizer<br>
>>  > <balazs.gibizer@est.tech> wrote:<br>
>>  >> Hi Neutrinos!<br>
>>  >><br>
>>  >>  We found a technical challenge during implementing the<br>
>>  >>  port-resource-request-groups API extension[1]. That extension<br>
>>  >> changes<br>
>>  >>  the format of the port.resoruce_request as well as the format <br>
>> of the<br>
>>  >>  port.binding:profile.allocation. The former is a calculated <br>
>> field on<br>
>>  >>  the port so that is easy. However the bindig:profile is <br>
>> persisted in<br>
>>  >>  the database so data migration is needed. What is the canonical <br>
>> way<br>
>>  >> to<br>
>>  >>  do such DB data translation in Neutron? Can we translate the <br>
>> data in<br>
>>  >>  place during alembic migration? Or should we do some kind of <br>
>> online<br>
>>  >>  data migration when the data is translated by neutron when it is<br>
>>  >> read<br>
>>  >>  from the db?<br>
>>  >><br>
>>  >>  cheers,<br>
>>  >>  gibi<br>
>>  >><br>
>>  >>  [1] <a href="https://review.opendev.org/c/openstack/neutron/+/805637/5" rel="noreferrer" target="_blank">https://review.opendev.org/c/openstack/neutron/+/805637/5</a><br>
>>  >><br>
>>  >><br>
>>  >><br>
>> <br>
>> <br>
<br>
<br>
</blockquote></div>