Skip to content
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

Update ScanServerRefFile format to sort on UUID #4691

Merged
merged 7 commits into from
Jul 24, 2024

Conversation

ddanielr
Copy link
Contributor

Updates the ScanServerRef format and moves the UUID entry forward so sorting is done on UUID instead of filename.
Also adds the removal of the scan serverref range from the metadata table

This PR is ensuring that the changes from #4682 are merged into main without introducing a large number of merge conflicts.

closes #4652 and #4493

@ddanielr
Copy link
Contributor Author

Currently in draft since I need to add tests for upgrade code and make sure existing tests still pass.

@@ -209,4 +216,16 @@ static void upgradeDataFileCF(final Key key, final Value value, final Mutation m
}
}

public void removeScanServerRange(ServerContext context, String tableName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upgrade code will also need to create the new table. In #4690 it creates the new table in the init code. Should try to reuse code for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found this in the elasticity upgrade code to create a fate table. But not sure if that is doing the same thing init does when it creates the fate table.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave #4652 open if the table creation is not done. Table creation in upgrade does not need to block this PR, we just need to ensure its not forgotten.

Copy link
Contributor

@keith-turner keith-turner Jun 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was looking into what the upgrade code in elasticity does for upgrade and it does not seem complete for creating the fate table. Opened #4692 and commented on #4652 after looking into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the table creation bits to this for zk table node creation, populating of table properties, and initial tablet file creation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to reuse the init code I had to make the class public.

I'm not a huge fan of that as a solution so I'm open to suggestions on where this table creation code should live.

@ddanielr ddanielr force-pushed the sserver-ref-change-main branch 4 times, most recently from 4db22dc to 63b898b Compare June 28, 2024 17:44
@ddanielr ddanielr marked this pull request as ready for review June 28, 2024 22:24
@cshannon
Copy link
Contributor

cshannon commented Jul 3, 2024

After we merge this forward into Elasticity, I think we could follow the same pattern here and use it to fix up the upgrade for the Fate table as part of #4692

@ddanielr ddanielr closed this Jul 5, 2024
@ddanielr ddanielr deleted the sserver-ref-change-main branch July 5, 2024 21:21
@ddanielr ddanielr restored the sserver-ref-change-main branch July 5, 2024 21:22
@ddanielr ddanielr reopened this Jul 5, 2024
@ctubbsii ctubbsii added this to the 3.1.0 milestone Jul 12, 2024
ddanielr and others added 7 commits July 24, 2024 19:55
Updates the ScanServerRef format and moves the UUID entry forward so
sorting is done on UUID instead of filename.

Also adds the removal of the scan serverref range from the metadata
table
* Moves the scanRefTablet creation to a separate method so it can be
  called from the upgrader code

* Handle scanRef table creation in Upgrader code

* Cleans up the FileSystemInitializer code

* Upgrader can now call the scanRef tablet creation codepath

* Separates the scanRef table node creation into a separate method that
  can be called from the upgrader code
ScanRefs are not written to the root table, only the metadata table so
handling scanref upgrades for the root table is not needed
* Removes the directory creation from the upgrade code
* Modifies the test to shutdown the cluster after removing the scan ref
table bits
@ddanielr ddanielr merged commit 15035d7 into apache:main Jul 24, 2024
8 checks passed
@ddanielr ddanielr deleted the sserver-ref-change-main branch July 24, 2024 20:11
@ctubbsii
Copy link
Member

@ddanielr this change has caused some test failures in org.apache.accumulo.test.ScanServerMetadataEntriesIT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle the addition of the new Scan Ref table during upgrade
4 participants