Codecov Report

Merging #6637 (7377bdf) into main (94b63f3) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #6637   +/-   ##
=======================================
  Coverage   93.01%   93.02%           
=======================================
  Files         259      259           
  Lines       55851    55852    +1     
=======================================
+ Hits        51952    51958    +6     
+ Misses       3899     3894    -5     

Do we need to remove the downgrades/ folder? The separation seems kind of useful given the large number of files in the sql directory.

Do we need to remove the downgrades/ folder?

No we do not need to do that. We can keep the folder.

The separation seems kind of useful given the large number of files in the sql directory.

I agree. However, for several reasons we did not place all the downgrade scripts in that folder.

How about keeping downgrades/ and making sure that all the upgrades are in sql/ and all the downgrades are in downgrades/?

How about keeping downgrades/ and making sure that all the upgrades are in sql/ and all the downgrades are in downgrades/?

seems reasonable

I removed all install-all targets as there is no need for them now.

I intentionally left install-downgrades target as it is. Older versions of Citus can still use that make target to install downgrade scripts in their clusters using that one.

I suggest we complete this PR after 11.2 release is over.

So what are the downgrade steps that people are supposed to follow? Is it this?

1. notice upgrade caused problems
2. run ALTER EXTENSION citus UPDATE 11.1-3
3. install 11.1 system packages

So what are the downgrade steps that people are supposed to follow? Is it this?

1. notice upgrade caused problems
2. run ALTER EXTENSION citus UPDATE 11.1-3
3. install 11.1 system packages

This list of actions is not possible without SET citus.enable_version_checks TO 'false';. I am not sure if we want an open source to mess around with this GUC.

I think it is still safer to do the following instead:

1. notice upgrade caused problems.
2. install 11.1 system packages
3. get a copy of the latest source code, and run make install-downgrades
4. Run ALTER EXTENSION citus UPDATE without supplying any versions.

The end user does not (and IMO should not) really care about the migration version, and they can easily break their systems if they migrate to a wrong version. For example, 11.1.5 uses migration version of 11.1-1. We do not even have the migration version 11.1-3 that you have in step 2.

A better solution would be maintaining a downgrade package that installs the latest downgrade scripts. If we had such a package, we could prefer the following actions:

1. notice upgrade caused problems. (you have the citus package and latest downgrade package as it is listed as a dependency)
2. install 11.1 system packages. (installing 11.1 packages do not touch the existing downgrade package installed in the system as the dependency is still satisfied)
3. run ALTER EXTENSION citus UPDATE without supplying any versions.

I think it is still safer to do the following instead:

1. notice upgrade caused problems.
2. install 11.1 system packages
3. get a copy of the latest source code, and run make install-downgrades
4. Run ALTER EXTENSION citus UPDATE without supplying any versions.

I think @marcocitus found some problem with this flow a while back, and that it didn't actually work in practice. And we never noticed because our tests don't actually do it, but instead use the SET citus.enable_version_checks TO 'false'; trick. @marcocitus could you share some context on this if you have that?

I would really like a flow that allows you to run ALTER EXTENSION citus UPDATE.

For context, the reason why downgrades from the downgrades directory are not installed by default is because we indeed had this citus-downgrades package plan (but so far we never did it). And if the files would be part of the main package, they would conflict with the ones from the downgrade package.

Iirc the columnar decoupling in 11.0 -> 11.1 necessitates that we do the ALTER EXTENSION citus UPDATE TO '11.0-4 from the 11.1 binaries, because it uses the utility hook to modify the behaviour of ALTER EXTENSION.

There was an issue downgrading from 11.0 to 10.2 #6041, though that was using 10.2 binaries for running the ALTER EXTENSION.