ctubbsii commented on a change in pull request #2185:
URL: https://github.com/apache/accumulo/pull/2185#discussion_r665671675
##########
File path:
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
##########
@@ -144,6 +150,11 @@ public void upgradeMetadata(ServerContext ctx) {
upgradeFileDeletes(ctx, Ample.DataLevel.USER);
}
+ @Override
+ public void upgradeFiles(ServerContext ctx) {
+ dropSortedMapWALFiles(ctx.getVolumeManager());
Review comment:
What happens when we delete these files? Wouldn't sorted files have
references in the metadata tables? And, if so, would their original WAL files
have already been subject to deletion? Is it safe to just remove these and
Accumulo will just recreate them using the new format? That behavior isn't
clear to me.
##########
File path: server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
##########
@@ -1120,6 +1122,9 @@ boolean canSuspendTablets() {
if (null != upgradeMetadataFuture) {
upgradeMetadataFuture.get();
}
+ if (null != upgradeFilesFuture) {
+ upgradeFilesFuture.get();
+ }
Review comment:
Does this imply that files can be upgraded concurrently with the
metadata? In general, this is probably not the case. It just happens to work
this time, because the files we want to upgrade are in one directory. But, I
worry about this pattern being added to the upgrade framework, because the
changes to the upgrade coordinator might imply that file upgrades are
independent of metadata upgrades, when usually files probably depend on the
content of the metadata (via the file references).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
