Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Possible fix and unit test. Fixes #961 #1048

Merged
merged 3 commits into from Mar 25, 2019
Merged

Possible fix and unit test. Fixes #961 #1048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9970132
Possible fix and unit test
milleruntime Mar 20, 2019
2dedbc7
Minimize changes to help with merge fwd
milleruntime Mar 21, 2019
ae0c826
Update Test
milleruntime Mar 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions server/tserver/src/main/java/org/apache/accumulo/tserver/log/RecoveryLogReader.java
View file
Open in desktop
Expand Up @@ -127,6 +127,9 @@ public RecoveryLogReader(VolumeManager fs, Path directory, LogFileKey start, Log
foundFinish = true;
continue;
}
if (SortedLogState.FAILED.getMarker().equals(child.getPath().getName())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add this condition as an OR to the first if stmt in the for loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking this is easier to remove, if that's what we plan on for 2.0.

continue;
}
FileSystem ns = fs.getVolumeByPath(child.getPath()).getFileSystem();
heap.add(new Index(new Reader(ns.makeQualified(child.getPath()), ns.getConf())));
}
Expand Down
25 changes: 25 additions & 0 deletions server/tserver/src/test/java/org/apache/accumulo/tserver/log/RecoveryLogsReaderTest.java
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@

import org.apache.accumulo.server.fs.VolumeManager;
import org.apache.accumulo.server.fs.VolumeManagerImpl;
import org.apache.accumulo.server.log.SortedLogState;
import org.apache.accumulo.tserver.log.RecoveryLogReader.SortCheckIterator;
import org.apache.accumulo.tserver.logger.LogEvents;
import org.apache.accumulo.tserver.logger.LogFileKey;
Expand Down Expand Up @@ -177,4 +178,28 @@ public void testSortCheck() {
}
}

/**
* Test a failed marker doesn't cause issues. See Github issue
* https://github.com/apache/accumulo/issues/961
*/
@Test
public void testFailed() throws Exception {
Path manyMaps = new Path("file://" + root.getRoot().getAbsolutePath() + "/manyMaps");
fs.create(new Path(manyMaps, SortedLogState.FAILED.getMarker())).close();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will other test see this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good question. I will check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a delete at the end of the Test


RecoveryLogReader reader = new RecoveryLogReader(fs, manyMaps);
IntWritable key = new IntWritable();
BytesWritable value = new BytesWritable();

for (int i = 0; i < 1000; i++) {
if (i == 10)
continue;
assertTrue(reader.next(key, value));
assertEquals(i, key.get());
}
reader.close();

assertTrue(fs.delete(new Path(manyMaps, SortedLogState.FAILED.getMarker())));
}

}