ctubbsii commented on a change in pull request #1803:
URL: https://github.com/apache/accumulo/pull/1803#discussion_r529776670
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/master/recovery/RecoveryPath.java
##########
@@ -18,13 +18,19 @@
*/
package org.apache.accumulo.server.master.recovery;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.cli.ServerUtilOpts;
import org.apache.accumulo.server.fs.VolumeManager.FileType;
import org.apache.hadoop.fs.Path;
public class RecoveryPath {
// given a wal path, transform it to a recovery path
public static Path getRecoveryPath(Path walPath) {
+
+ ServerUtilOpts opts = new ServerUtilOpts();
+ ServerContext context = opts.getServerContext();
Review comment:
This isn't the right way to get the ServerContext. The caller should
pass in the context reference that they have, as an additional parameter to
this method. I see two callers for this method. `TabletServer` should have a
`getContext()` method on it, and `RecoveryManager` can use
`master.getContext()`.
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/master/recovery/RecoveryPath.java
##########
@@ -42,7 +48,8 @@ public static Path getRecoveryPath(Path walPath) {
// drop wal
walPath = walPath.getParent();
- walPath = new Path(walPath, FileType.RECOVERY.getDirectory());
+ walPath = new Path(walPath,
+ FileType.RECOVERY.getDirectory() + '/' +
context.getUniqueNameAllocator().getNextName());
Review comment:
It would be good to log a message about the name that was chosen, before
being used here, as a debug message, so that a an admin can trace the specific
recovery directory back to a specific server from the server logs if necessary.
Instead of using `'/'`, you can use `'-'` so that way the directories look
like `recovery-<uniquePart>`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
