openbmc / technical-oversight-forum

6 stars 1 forks source link

RFC: Dealing with Backward Compatibility #13

Open williamspatrick opened 2 years ago

williamspatrick commented 2 years ago

In https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-state-manager/+/52262 it was pointed out that a code change was going to change the format of data held in persistence which would cause this data to be lost for existing files in production. It was also mentioned by the maintainer that there isn't a good policy stance for the project on if this is acceptable or not in the master branch.

Can we set a direction here?

  1. Master requires no backwards compatibility of persistent data; this is for the system owners to worry about.
  2. Master cannot break backwards compatibility of persistent data.
  3. We will set no direction.

If we decide on (2) we should also find someone willing to work on a migration pattern that can be easily added to recipes and/or repositories to deal with backwards compatibility migrations.

We should also be clear on what our stance is with "forwards compatibility". Meaning, what happens if I upgrade from A to B and a data-migration was done, but the system ends up running on version A again later. Are we expecting that migrations only push the data forward into new files and do not delete the old ones?

I've titled this "RFC:" because I'd like some feedback before I spend more time refining the wording for a formal proposal.

edtanous commented 2 years ago

Inside the repos I maintain that store state (bmcweb, entity-manager) I have required patches maintain number 2 above, to within the limits of which engineering allows (ie, a major subsystem change might not have a way to pick intelligent defaults, in which case it should resolve the conflict in the most user friendly way possible). In practice, it tends to be a trivial amount of code difference in size, and is largely just a matter of understanding the need for schema changes, not changing key names and filenames for arbitrary reasons, not failing to parse on unknown keys, and when we do change, we write a "port the key forward".

Admittedly, this is highly subjective, and there are degrees to which the user impact can be measured. In the above example, resetting the POH counter to zero for all existing systems seems like it would defeat most of the purpose of the feature, given that it's intended for managing system longevity, so that seems like a bad thing to do.

Vote for 2, but with a little more specific phrasing "Master cannot break backwards compatibility of persistent data in a user visible way, for features that have existed on master through a versioned release."

This gives us the ability to make transitions going forward, roots the requirement on user-facing impact and gives us the ability to say, fix a bug that has only existed on master for a week, where changing would have likely no impact.

leiyu-bytedance commented 2 years ago

Vote for 2, but with a little more specific phrasing "Master cannot break backwards compatibility of persistent data in a user visible way, for features that have existed on master through a versioned release."

+1 for this, except for the case that it shall provide a way to transform the persistent data into the new format. E.g. the service could look for old format, load it, and store the data with the new format, this makes sure a smooth transition.

bradbishop commented 2 years ago

Vote for 2, but with a little more specific phrasing "Master cannot break backwards compatibility of persistent data in a user visible way, for features that have existed on master through a versioned release."

Agreed. On forward compatibility:

Are we expecting that migrations only push the data forward into new files and do not delete the old ones?

It would be nice if we could provide this level of service.

nest1ing commented 2 years ago

I've thought about the integration of upgrade/downgrade scripts into the firmware bundle. These scripts could be called by phsosphor-software-manager during the firmware activation and can support both directions (upgrade and downgrade) and may be customized for vendor specific actions.

But I didn't try to implement it yet, so it's still just an idea.

joseph-reynolds commented 2 years ago

Notes: