Handle old sorted map files in Upgrade #2185

milleruntime · 2021-06-30T17:35:25Z

* Add upgradeFiles method for upgrading files to Upgrader
* Refactor status handling in UpgradeCoordinator to allow upgradeFiles
to run concurrently and wait for metadata upgrade to complete
* Implement upgradeFiles method in Upgrader9to10

Add dropSortedMapWALFiles for resolving sorted map files that may
still be around during upgrade. Follow up for Make sorted recovery write to RFiles #2117
Closes Handle old sorted map files #2179

Manno15

Everything looks good. Noticed one small typo but that is about it.

server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java

server/manager/src/main/java/org/apache/accumulo/manager/Manager.java

server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java

Manno15

Everything looks good from my perspective. Have you done any testing to verify that Accumulo will automatically re-sort the WALs once these are deleted? Any way to create a test for that?

milleruntime · 2021-07-30T16:18:22Z

Everything looks good from my perspective. Have you done any testing to verify that Accumulo will automatically re-sort the WALs once these are deleted? Any way to create a test for that?

Yeah I did some manual testing with Uno. It may be possible to create an IT with the old files saved off but I imagine it would be quite involved. I am not sure its worth the work for a one time thing that will only ever happen between versions.

* Add upgradeFiles method for upgrading files to Upgrader * Refactor status handling in UpgradeCoordinator to allow upgradeFiles to run concurrently and wait for metadata upgrade to complete * Implement upgradeFiles method in Upgrader9to10 * Add dropSortedMapWALFiles for resolving sorted map files that may still be around during upgrade. Follow up for apache#2117 * Closes apache#2179

milleruntime · 2021-07-30T18:31:15Z

@ctubbsii I reverted the changes I made adding a new upgradeFiles method to the Upgrade interface and now this change is much simpler. I believe I got rid of the changes you had concern over. This PR was just the new code I added to fix old sorted logs.

milleruntime · 2021-07-30T20:21:08Z

So I did so more testing and while it successfully deleted the old data, I am not 100% sure that it won't delete WALs that still have referenced data in them. This is a little tricky to test manually, as you need to make sure the table is recovering but also have unloaded tablets (before the data is flushed to disk). I am going to open a ticket to test this using Continuous Ingest.

keith-turner · 2021-08-04T14:35:21Z

server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java

+   * Remove old temporary map files to prevent problems during recovery.
+   */
+  static void dropSortedMapWALFiles(VolumeManager vm) {
+    Path recoveryDir = new Path("/accumulo/recovery");


Seems like this should loop over the set of configured volumes instead of this.

Good catch. I will create a follow on ticket with this and any other suggestions you have.

Do you think using the volumes configured in instance.volumes will be enough?

Do you think using the volumes configured in instance.volumes will be enough?

Yeah, could look through all of those. Could narrow it by calling :

accumulo/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java

Line 180 in a9100ac

Set<String> choosable(org.apache.accumulo.core.spi.fs.VolumeChooserEnvironment env,

But I don't think it hurts to just look through all volumes. Maybe could use

accumulo/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java

Line 196 in a9100ac

Collection<Volume> getVolumes();

to get the list of volumes

Would probably be best to avoid choosable() as that will give the volumes configured for new WALs. The config could change and an old WAL could be on a volume that choosable() no longer returns. So probably best to inspect all volumes looking for old sorted wals to nuke.

Would probably be best to avoid choosable() as that will give the volumes configured for new WALs. The config could change and an old WAL could be on a volume that choosable() no longer returns. So probably best to inspect all volumes looking for old sorted wals to nuke.

Definitely avoid this. Consider RandomVolumeChooser 😺

I thought the VolumeChooser was for selecting volumes for writes? I want to loop through every volume configured to find the old sorted WALs to remove. Will the VolumeChooser allow for this?

keith-turner · 2021-08-04T14:39:13Z

server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java

@@ -142,6 +148,8 @@ public void upgradeMetadata(ServerContext ctx) {
    upgradeRelativePaths(ctx, Ample.DataLevel.USER);
    upgradeDirColumns(ctx, Ample.DataLevel.USER);
    upgradeFileDeletes(ctx, Ample.DataLevel.USER);
+    // special case where old files need to be deleted


Did you consider calling this new method in upgradeZookeeper instead? I think that is called before the root tablet is loaded, which would allow deleting any old sorted logs the root tablet may reference.

No, I didn't think any tables were loaded until after the Upgrader was finished. If that is the case, then moving it to upgradeZookeeper would be better.

I think the upgrade methods are run as follows.

upgradeZookeeper() is run before the root tablet is loaded.

upgradeRoot() is run after the root tablet is loaded and before the metadata table is loaded.

upgradeMetadata is run after the metadata table is loaded and before loading user tablets.

Ah OK. I will move it to upgradeZookeeper().

Capture discussion on PR apache#2185 to update javadoc comments. Doc update only - contains no code changes.

…2225) * Update interface comments to include expected system state. Capture discussion on PR #2185 to update javadoc comments. Doc update only - contains no code changes. * update phrasing per PR comments Co-authored-by: Ed Coleman etcoleman <edcoleman@apache.org>

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles() * Add comments to Upgrader interface

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

…pache#2225) * Update interface comments to include expected system state. Capture discussion on PR apache#2185 to update javadoc comments. Doc update only - contains no code changes. * update phrasing per PR comments Co-authored-by: Ed Coleman etcoleman <edcoleman@apache.org>

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

* Follow on work for #2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

Manno15 approved these changes Jun 30, 2021

View reviewed changes

Manno15 reviewed Jun 30, 2021

View reviewed changes

server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java Outdated Show resolved Hide resolved

milleruntime requested a review from keith-turner July 6, 2021 13:48

ctubbsii reviewed Jul 7, 2021

View reviewed changes

server/manager/src/main/java/org/apache/accumulo/manager/Manager.java Outdated Show resolved Hide resolved

server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java Show resolved Hide resolved

ctubbsii added this to In progress in 2.1.0 via automation Jul 7, 2021

milleruntime requested a review from Manno15 July 30, 2021 14:41

Manno15 approved these changes Jul 30, 2021

View reviewed changes

milleruntime added 3 commits July 30, 2021 12:20

Fix typo

9eb10dc

Revert new upgradeFiles and put method call in metadata upgrade

03e72b3

milleruntime force-pushed the sorted-wal-fix branch from 4a51d7b to 03e72b3 Compare July 30, 2021 16:21

milleruntime changed the title ~~Add upgradeFiles to Upgrade code~~ Handle old sorted map files in Upgrade Jul 30, 2021

milleruntime merged commit 09e3d77 into apache:main Jul 30, 2021

2.1.0 automation moved this from In progress to Done Jul 30, 2021

milleruntime deleted the sorted-wal-fix branch July 30, 2021 16:43

milleruntime mentioned this pull request Jul 30, 2021

Test temp Sorted WALs are handled properly on Upgrade using CI #2218

Closed

keith-turner reviewed Aug 4, 2021

View reviewed changes

EdColeman pushed a commit to EdColeman/accumulo that referenced this pull request Aug 4, 2021

Update interface comments to include expected system state.

f74aa00

Capture discussion on PR apache#2185 to update javadoc comments. Doc update only - contains no code changes.

EdColeman mentioned this pull request Aug 4, 2021

Update Upgrader interface comments to include expected system state. #2225

Merged

milleruntime added a commit to milleruntime/accumulo that referenced this pull request Aug 12, 2021

Rework sorted WAL files Upgrade code

dbacf47

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles() * Add comments to Upgrader interface

milleruntime added a commit to milleruntime/accumulo that referenced this pull request Aug 12, 2021

Rework sorted WAL files Upgrade code

cfb46d1

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

milleruntime mentioned this pull request Aug 12, 2021

Rework sorted WAL Upgrade code #2231

Merged

milleruntime added a commit to milleruntime/accumulo that referenced this pull request Aug 13, 2021

Rework sorted WAL files Upgrade code

dd16633

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

milleruntime added a commit to milleruntime/accumulo that referenced this pull request Aug 18, 2021

Rework sorted WAL files Upgrade code

2881702

* Follow on work for apache#2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

milleruntime added a commit that referenced this pull request Aug 23, 2021

Rework sorted WAL Upgrade code (#2231)

87d4aa5

* Follow on work for #2185 * Improve upgrade code in Upgrader9to10.dropSortedMapWALFiles()

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle old sorted map files in Upgrade #2185

Handle old sorted map files in Upgrade #2185

milleruntime commented Jun 30, 2021 •

edited

Manno15 left a comment

Manno15 left a comment

milleruntime commented Jul 30, 2021

milleruntime commented Jul 30, 2021

milleruntime commented Jul 30, 2021

keith-turner Aug 4, 2021

milleruntime Aug 4, 2021

milleruntime Aug 4, 2021

keith-turner Aug 4, 2021 •

edited

keith-turner Aug 4, 2021

ctubbsii Aug 4, 2021

milleruntime Aug 11, 2021

keith-turner Aug 4, 2021

milleruntime Aug 4, 2021

keith-turner Aug 4, 2021

milleruntime Aug 4, 2021

Handle old sorted map files in Upgrade #2185

Handle old sorted map files in Upgrade #2185

Conversation

milleruntime commented Jun 30, 2021 • edited

Manno15 left a comment

Choose a reason for hiding this comment

Manno15 left a comment

Choose a reason for hiding this comment

milleruntime commented Jul 30, 2021

milleruntime commented Jul 30, 2021

milleruntime commented Jul 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keith-turner Aug 4, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milleruntime commented Jun 30, 2021 •

edited

keith-turner Aug 4, 2021 •

edited