ctubbsii commented on a change in pull request #2181:
URL: https://github.com/apache/accumulo/pull/2181#discussion_r665733606
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -123,13 +127,18 @@ public void execute(String[] args) throws Exception {
Set<Integer> tabletIds = new HashSet<>();
for (String file : opts.files) {
-
Path path = new Path(file);
LogFileKey key = new LogFileKey();
LogFileValue value = new LogFileValue();
+ // read log entries from a single WAL file.
if (fs.getFileStatus(path).isFile()) {
- // read log entries from a simple hdfs file
+ if (file.endsWith(".rf")) {
+ log.error(
+ "Can not read from a single rfile. Please pass in a directory
for recovery logs.");
Review comment:
```suggestion
"Unable to read from a single RFile. A non-sorted WAL file
was expected. To read sorted WALs, please pass in a directory containing the
sorted recovery logs.");
```
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java
##########
@@ -123,12 +127,16 @@ public void execute(String[] args) throws Exception {
Set<Integer> tabletIds = new HashSet<>();
for (String file : opts.files) {
-
Path path = new Path(file);
LogFileKey key = new LogFileKey();
LogFileValue value = new LogFileValue();
if (fs.getFileStatus(path).isFile()) {
+ if (file.endsWith(".rf")) {
Review comment:
Based on the updated comments, I can see that the first case is for
reading regular non-sorted WAL files, and not the sorted ones, whereas the
`else` block is for reading sorted WAL files from a single directory. I think
that could be made even more clear:
```suggestion
// make sure it's a regular non-sorted WAL file, and not a single
sorted WAL in RFile format
if (file.endsWith(".rf")) {
```
Thinking about this a bit, it's a little weird that we can't read a single
sorted WAL file. We can read a directory containing one, but we can't read it
by itself? That doesn't seem to be a useful restriction for this utility, which
is primarily intended to be a troubleshooting utility. If we pass in a single
RFile, I feel like this tool should still be able to read it. That could be
done here or in a separate PR, unless there's a reason to not support that at
all.
--
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]
