apache / accumulo

Apache Accumulo
https://accumulo.apache.org
Apache License 2.0
1.06k stars 444 forks source link

Create fate table in upgrade code #4692

Open keith-turner opened 2 weeks ago

keith-turner commented 2 weeks ago

The fate table needs to be created in the upgrade process. It should be created exactly the same way as the init code creates the table. Ideally upgrade and init would share code.

keith-turner commented 2 weeks ago

There seems to be some existing code that creates the fate table in upgrade. However it does not seem to do everything the init code does like set some initial table properties.

keith-turner commented 2 weeks ago

The following is some code that I found that init runs, but could not find upgrade running. Possible I missed something.

https://github.com/apache/accumulo/blob/46d8c0b71deb4e7287eff0158e7014247f929ae2/server/base/src/main/java/org/apache/accumulo/server/init/FileSystemInitializer.java#L179

cshannon commented 1 week ago

Besides setting properties there seems to be a couple more things that would need to be done during upgrade that FileSystemInitializer is handling for a new system. All of it is driven by the FileSystemInitializer initialize() method:

  1. We need to create the new directories for the new table which is done here and here
  2. We need to add entries to the metadata table for the new Fate tablet which is done here

@keith-turner - I am not sure if I am missing anything else or if that is it. I am thinking we might need to do some refactoring with the initializer so that the upgrade code can also use it. Because the new Scan ref table is used in both 3.1 and elasticity maybe that should be done first in #4652 and include some refactoring, and then we can update elasticity upgrade to use it for Fate as well. It might be tricky to do both tables together in the same commit with Fate only applying to elasticity.

ddanielr commented 6 days ago

@cshannon The last piece you need is the table node creation in ZK. That is handled within the ZooKeeperInitializer here.

If this isn't called then the setTableProperties method will fail because the top-level table entry in ZK doesn't exist yet.

I agree that the init code needs to be refactored so both upgrade and init can use it.

keith-turner commented 5 days ago

I plan to get the test in #4717 and #4718 working with 2.1. Then will have a way to test upgrade from 2.1->3.1 and 2.1->4.0. Once that is in place can make the 2.1 code create some scan refs prior to upgrade and verify in 3.1 and 4.0 that they are deleted after upgrade.