Skip to content

rfc: how to upgrade clusters with enhanced view descriptors #17502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

knz
Copy link
Contributor

@knz knz commented Aug 8, 2017

Needed to solve #10023 / #10083.
Complement to #17501.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz requested review from a team, danhhz and a-robinson August 8, 2017 13:34
@knz
Copy link
Contributor Author

knz commented Aug 8, 2017

@danhhz if there are any considerations I'm missing about bulk I/O here, please chime in.

@knz knz force-pushed the 20170808-view-migrations branch from 8d610d0 to bc0ed1d Compare August 8, 2017 13:39
@knz knz force-pushed the 20170808-view-migrations branch from bc0ed1d to b51f72a Compare August 8, 2017 13:41
@bdarnell
Copy link
Contributor

bdarnell commented Aug 8, 2017

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


docs/RFCS/view_dependencies.md, line 200 at r1 (raw file):

## Break views for non-upgraded nodes.

1. start one node with 1.2; this runs a migration which rewrites all view descriptors, *and bumps their FormatVersion* field.

I think we should do something like this, but instead of triggering it when the first node is started with 1.2 (as currently happens for the migrations/sqlmigrations package), it should happen when the upgrade command from #16977 is issued. That way we know that (the operator asserts that) there will be no more nodes still running 1.1.


Comments from Reviewable

@a-robinson
Copy link
Contributor

I'm sorry for letting this slip, @knz. I'll be happy to take a look at it if you or anyone else gets the drive to finish it.

@knz
Copy link
Contributor Author

knz commented Dec 19, 2017

it would be nice, also Toby has some thoughts on this topic. I'd be glad to receive more detailed technical input. However with my EOY schedule it will need to wait post-1.2 unfortunately.

@tbg
Copy link
Member

tbg commented Dec 19, 2017

With the cluster migration system that's in place, this doesn't seem impossible to do. There are two pieces, namely getting onto the new version that uses the new views and, perhaps more difficult, getting to a point where the old code can be removed.

Let's assume the new views are introduced in v1.5 (for sake of the example). So a cluster is running v1.4 before being upgraded, happily using the old views. To upgrade, the operator restarts the binary into v1.5 one after another. As long as the cluster version remains at 1.4, the new nodes will assume that all views follow the old code, i.e. have the SQL statement in the descriptor. In particular, creating a new view will create an old-style one.

When the operator issues SET CLUSTER SETTING version = '1.5', they promise that no more nodes remain at the old version. As the nodes (all of which are thus the new binary) receive this new cluster version, they are free to create new views using the new mechanism, and to upgrade existing views.

For creating new views, this just means that the code needs to be feature gated like this:

func dummyCreateView() {
  if !settings.IsActive(cluster.VersionBetterViews) {
      return createOldStyleView()
  }
  return createBetterView()
}

Upgrading the existing views comes with the slight problem that there's not necessarily anything that would trigger it today. This isn't really a problem until we want to get rid of the old code, which is discussed next.

We can remove the old code once we know for a fact that a) no old view remains and b) no old view will be created in the future.

b) is easier, but still not trivial. Nodes at v1.5 still can create old-style views except that they won't when they know that the cluster is also at v1.5. Instead of trying to figure out when all nodes know about that (typically very quickly, but it's an awkward problem to solve authoritatively since nodes can join the cluster at any moment), we can just use the fact that when the cluster runs at v1.6 (i.e after the next upgrade cycle) definitely no nodes will create old-style views assuming we're OK phasing out this code rather slowly over two releases.

a) is annoying but similarly solvable. Taking b) into account, we want to make sure that when the cluster is upgraded from v1.5 to v1.6, it does not leave an old-style view behind. So we want to add a pre-accept hook to the SET CLUSTER SETTING effects. In pseudo-code (and using functionality that is so far fictional) we would do something early in the server boot sequence:

func bootServer() {
  s := &Server{}
  s.st.Version.RegisterPreHook(func(_ irrelevantStuff) error {
    return s.sqlServer.upgradeAllOldStyleViews()
  })
}

Similarly, of course, we would upgrade any old view that is being touched so that views that are used are typically upgraded eagerly (so that users see the benefits on 1.5, not later):

func codeThatReadsAView(view View) {
  doStuff(view)
  if isOldStyle(view) {
    triggerConversionInBackground(view)
  }
}

So in short we have the following behavior:

  • cluster at 1.4: knows only old views (though any new binaries in the cluster understand new views, they won't use that functionality as they won't create any new ones until the cluster version bump)
  • cluster at 1.5: knows both, old views may be created by nodes that don't know the cluster version has been bumped; will optimistically convert views to new (doing it for all views right after the bump). In effect, old code is dead except for extraordinary circumstances and short blips of time just after having upgraded from 1.4.
  • binary 1.6: forgets all the logic for the old views, but introduces a RegisterPreHook that fails when an old-style view exists. The user would have to run the 1.5 binary (which upgrades the view) and upgrade again. This wouldn't reasonably be encountered by users, ever.
  • binary 1.7: remove the hook and forget about old views altogether.

For testing, the following plan comes to mind:

  1. cluster with binaries (1.4, 1.4, 1.4), create a view vold14
  2. upgrade to (1.4, 1.4, 1.5)
  3. create view vold15 at 1.5 and read all views from all nodes
  4. upgrade to (1.5, 1.5, 1.5) (still cluster version 1.4)
  5. create new view, read all views from all nodes
  6. downgrade to (1.4, 1.4, 1.4) and read everything
  7. upgrade back to (1.5, 1.5, 1.5) and issue SET CLUSTER SETTING version = '1.5'
  8. check that within a short amount of time, all views become "new-style" (perhaps by altering the table and seeing that they work?)
  9. when 1.6 is developed, test the pre-hook in unit tests. This should be easy: get all views, iterate over them and when one of them is "old", return an error.

@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label Jun 19, 2019
@tbg tbg removed the request for review from a team September 23, 2019 10:32
@knz
Copy link
Contributor Author

knz commented Dec 28, 2022

@mgartner @postamar @chengxiong-ruan same question as the other one. Can you check if there's anything salvageable here before i close this. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants