Skip to content

Conversation

@ddanielr
Copy link
Contributor

This approach handles rollbacks from 2.1.3 to 2.1.2 gracefully by allowing scan server references to exist in multiple sections with different storage formats and allows the ScanServerRefTabletFile to handle the logic internally.

The upgrade code now removes all scan server refs on upgrade to ensure the old scan server section is empty.
However, I'm not sure if that code will be run in the future since the AccumuloDataVersion has already been incremented in later versions.

  • Moves the scan server refs to a different prefix to avoid compatibility issues with possible downgrades
  • Modifies the Scan Server Reference format to put the UUID first in the row to reduce metadata tablet contention on writes
  • Adds a new "OldScanServerReferences" section to the metadata schema to ensure that future changes keep track of the previously used prefixes.

* Moves the scan server refs to a different prefix to avoid compatiblity
* issues with possible downgrades

* Modifies the Scan Server Reference format to put the UUID
  first in the row to reduce metadata tablet contention on writes

* Upgrade code now removes all scan server refs on upgrade to ensure old
* scan server section is deleted
@ddanielr
Copy link
Contributor Author

I think I might need to pull the upgrade code changes out and merge them into 3.1.

Cleaning out the references makes upgrades easier as we are removing transitive info from the metadata table that shouldn't persist through an upgrade. However, removal of the old scan server ref metadata section has to happen in 3.1.

@ddanielr
Copy link
Contributor Author

This PR closes #4493

@dlmarion
Copy link
Contributor

I'm wondering if #4650 (new scan server ref table) should be backported to 3.1. This would mean that in the upgrade from 2.1 to 3.1 the new table would be created and the old and new refs in the metadata table would be cleaned up.

@ddanielr
Copy link
Contributor Author

I'm wondering if #4650 (new scan server ref table) should be backported to 3.1. This would mean that in the upgrade from 2.1 to 3.1 the new table would be created and the old and new refs in the metadata table would be cleaned up.

That seems like a nice clean way to handle the transition. Are there any downsides to doing that?

@dlmarion
Copy link
Contributor

I'm wondering if #4650 (new scan server ref table) should be backported to 3.1. This would mean that in the upgrade from 2.1 to 3.1 the new table would be created and the old and new refs in the metadata table would be cleaned up.

That seems like a nice clean way to handle the transition. Are there any downsides to doing that?

I'm sure that it won't apply cleanly, but in theory should be do-able. Probably a good question for @cshannon .

@keith-turner
Copy link
Contributor

I'm wondering if #4650 (new scan server ref table) should be backported to 3.1. This would mean that in the upgrade from 2.1 to 3.1 the new table would be created and the old and new refs in the metadata table would be cleaned up.

That seems like a nice clean way to handle the transition. Are there any downsides to doing that?

I do not know of any downsides. Backporting that does seems nice because it would avoid having slightly different ways of doing things in the three branches.

@cshannon
Copy link
Contributor

I'm sure that it won't apply cleanly, but in theory should be do-able. Probably a good question for @cshannon .

I can't really think of an issues with backporting the new table other than the merge conflicts as alluded to so would need to work through that.

The main thing would be seeing if there's stuff that only exists in elasticity that the new code uses and would also need to move. I would need to look more at it but the main thing off the top of my head is there were some changes to the table initialization code that was added for new table initialization and there might be some missing test stuff too.

ddanielr added 3 commits June 20, 2024 18:35
Backported the UuidUtil class from commit (e68f9dd)
* Added validation of stored ScanServerRefs to ensure they match the
  desired prefix

* Changed prefix to relate to scan servers instead of just scans
* Removes the scan server ref upgrade code changes as they won't be run.
  These will be moved to the 3.1 release version.
ddanielr and others added 3 commits June 20, 2024 20:17
…rRefTabletFile.java


Avoid string conversion

Co-authored-by: Keith Turner <kturner@apache.org>
Moves the row creation from ample into the ScanServerRefTabletFile
@cshannon
Copy link
Contributor

@ddanielr and @dlmarion - I'm going to go start work on a PR today to back port the new table from #4650 to 3.1. I think it makes sense to do that for the upgrade.

@cshannon
Copy link
Contributor

PR for the backport is #4690

@dlmarion
Copy link
Contributor

When this is merged into 2.1, what's the merge strategy into main given #4690?

@cshannon
Copy link
Contributor

When this is merged into 2.1, what's the merge strategy into main given #4690?

I would suggest working out the merge conflicts in another branch for main before merging to 2.1 so it is ready to go and then the changes can be applied during the merge forward.

@ddanielr
Copy link
Contributor Author

Started working on the merge conflicts with #4690 and since that PR removes the prefix entirely, all of these changes to handle various prefixes aren't needed.

So the 2.1 -> main branch merge containing these changes will be a noop merge and I'll address the row format change with the new upgrade code in a PR to close out #4652.

@ddanielr ddanielr merged commit cf3792a into apache:2.1 Jun 21, 2024
@ddanielr ddanielr deleted the NewScanServerRefPrefix branch June 21, 2024 21:04
@ctubbsii ctubbsii modified the milestones: 3.1.0, 2.1.3 Jul 12, 2024
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.

5 participants