Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/107#discussion_r65652662
--- Diff:
server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java
---
@@ -234,4 +257,288 @@ public void testIsUUID() {
assertFalse(GarbageCollectWriteAheadLogs.isUUID("0" +
UUID.randomUUID().toString()));
assertFalse(GarbageCollectWriteAheadLogs.isUUID(null));
}
+
+ @Test
+ public void testTimeToDeleteTrue() throws InterruptedException {
+ gcwal.clearFirstSeenDead();
+ HostAndPort address = HostAndPort.fromString("tserver1:9998");
+ long wait = AccumuloConfiguration.getTimeInMillis("1s");
+ gcwal.timeToDelete(address, wait); // to store first seen dead
+ sleep(wait * 2);
+ assertTrue(gcwal.timeToDelete(address, wait));
+ }
+
+ @Test
+ public void testTimeToDeleteFalse() {
+ HostAndPort address = HostAndPort.fromString("tserver1:9998");
+ long wait = AccumuloConfiguration.getTimeInMillis("1h");
+ long t1, t2;
+ boolean ttd;
+ do {
+ t1 = System.nanoTime();
+ gcwal.clearFirstSeenDead();
+ gcwal.timeToDelete(address, wait);
+ ttd = gcwal.timeToDelete(address, wait);
+ t2 = System.nanoTime();
+ } while (t2 - t1 > (wait / 2) * 1000000); // as long as it took less
than half of the configured wait
+
+ assertFalse(gcwal.timeToDelete(address, wait));
+ }
+
+ @Test
+ public void testTimeToDeleteWithNullAddress() {
+ assertFalse(gcwal.timeToDelete(null, 123l));
+ }
+
+ /**
+ * Wrapper class with some helper methods
+ * <p>
+ * Just a wrapper around a LinkedHashMap that store method name and
argument information. Also includes some convenience methods to make usage
cleaner.
+ */
+ class MethodCalls {
+
+ private LinkedHashMap<String,List<Object>> mapWrapper;
+
+ public MethodCalls() {
+ mapWrapper = new LinkedHashMap<String,List<Object>>();
+ }
+
+ public void put(String methodName, Object... args) {
+ mapWrapper.put(methodName, Arrays.asList(args));
+ }
+
+ public int size() {
+ return mapWrapper.size();
+ }
+
+ public boolean hasOneEntry() {
+ return size() == 1;
+ }
+
+ public Map.Entry<String,List<Object>> getFirstEntry() {
+ return mapWrapper.entrySet().iterator().next();
+ }
+
+ public String getFirstEntryMethod() {
+ return getFirstEntry().getKey();
+ }
+
+ public List<Object> getFirstEntryArgs() {
+ return getFirstEntry().getValue();
+ }
+
+ public Object getFirstEntryArg(int number) {
+ return getFirstEntryArgs().get(number);
+ }
+ }
+
+ /**
+ * Partial mock of the GarbageCollectWriteAheadLogs for testing the
removeFile method
+ * <p>
+ * There is a map named methodCalls that can be used to assert
parameters on methods called inside the removeFile method
+ */
+ class GCWALPartialMock extends GarbageCollectWriteAheadLogs {
+
+ private boolean holdsLockBool = false;
+
+ public GCWALPartialMock(Instance i, VolumeManager vm, boolean
useTrash, boolean holdLock) throws IOException {
+ super(i, vm, useTrash);
+ this.holdsLockBool = holdLock;
+ }
+
+ public MethodCalls methodCalls = new MethodCalls();
+
+ @Override
+ boolean holdsLock(HostAndPort addr) {
+ return holdsLockBool;
+ }
+
+ @Override
+ void removeWALfromDownTserver(HostAndPort address,
AccumuloConfiguration conf, Entry<String,ArrayList<Path>> entry, final GCStatus
status) {
+ methodCalls.put("removeWALFromDownTserver", address, conf, entry,
status);
+ }
+
+ @Override
+ void askTserverToRemoveWAL(HostAndPort address, AccumuloConfiguration
conf, Entry<String,ArrayList<Path>> entry, final GCStatus status) {
+ methodCalls.put("askTserverToRemoveWAL", address, conf, entry,
status);
+ }
+
+ @Override
+ void removeOldStyleWAL(Entry<String,ArrayList<Path>> entry, final
GCStatus status) {
+ methodCalls.put("removeOldStyleWAL", entry, status);
+ }
+
+ @Override
+ void removeSortedWAL(Path swalog) {
+ methodCalls.put("removeSortedWAL", swalog);
+ }
+ }
+
+ private GCWALPartialMock getGCWALForRemoveFileTest(GCStatus s, final
boolean locked) throws IOException {
+ return new GCWALPartialMock(new MockInstance("accumulo"),
VolumeManagerImpl.get(), false, locked);
+ }
+
+ private Map<String,Path> getEmptyMap() {
+ return new HashMap<String,Path>();
+ }
+
+ private Map<String,ArrayList<Path>> getServerToFileMap1(String key, Path
singlePath) {
+ Map<String,ArrayList<Path>> serverToFileMap = new
HashMap<String,ArrayList<Path>>();
+ serverToFileMap.put(key, new
ArrayList<Path>(Arrays.asList(singlePath)));
+ return serverToFileMap;
+ }
+
+ @Test
+ public void testRemoveFilesWithOldStyle() throws IOException {
+ GCStatus status = new GCStatus();
+ GarbageCollectWriteAheadLogs realGCWAL =
getGCWALForRemoveFileTest(status, true);
+ Path p1 = new Path("hdfs://localhost:9000/accumulo/wal/tserver1+9997/"
+ UUID.randomUUID().toString());
+ Map<String,ArrayList<Path>> serverToFileMap = getServerToFileMap1("",
p1);
+
+ realGCWAL.removeFiles(getEmptyMap(), serverToFileMap, getEmptyMap(),
status);
+
+ MethodCalls calls = ((GCWALPartialMock) realGCWAL).methodCalls;
+ assertEquals("Only one method should have been called", 1,
calls.size());
+ assertEquals("Method should be removeOldStyleWAL",
"removeOldStyleWAL", calls.getFirstEntryMethod());
+ Entry<String,ArrayList<Path>> firstServerToFileMap =
serverToFileMap.entrySet().iterator().next();
+ assertEquals("First param should be empty", firstServerToFileMap,
calls.getFirstEntryArg(0));
+ assertEquals("Second param should be the status", status,
calls.getFirstEntryArg(1));
+ }
+
+ @Test
+ public void testRemoveFilesWithDeadTservers() throws IOException {
+ GCStatus status = new GCStatus();
+ GarbageCollectWriteAheadLogs realGCWAL =
getGCWALForRemoveFileTest(status, false);
+ String server = "tserver1+9997";
+ Path p1 = new Path("hdfs://localhost:9000/accumulo/wal/" + server +
"/" + UUID.randomUUID().toString());
+ Map<String,ArrayList<Path>> serverToFileMap =
getServerToFileMap1(server, p1);
+
+ realGCWAL.removeFiles(getEmptyMap(), serverToFileMap, getEmptyMap(),
status);
+
+ MethodCalls calls = ((GCWALPartialMock) realGCWAL).methodCalls;
+ assertEquals("Only one method should have been called", 1,
calls.size());
+ assertEquals("Method should be removeWALfromDownTserver",
"removeWALFromDownTserver", calls.getFirstEntryMethod());
+ assertEquals("First param should be address",
HostAndPort.fromString(server.replaceAll("[+]", ":")),
calls.getFirstEntryArg(0));
+ assertTrue("Second param should be an AccumuloConfiguration",
calls.getFirstEntryArg(1) instanceof AccumuloConfiguration);
+ Entry<String,ArrayList<Path>> firstServerToFileMap =
serverToFileMap.entrySet().iterator().next();
+ assertEquals("Third param should be the entry", firstServerToFileMap,
calls.getFirstEntryArg(2));
+ assertEquals("Forth param should be the status", status,
calls.getFirstEntryArg(3));
+ }
+
+ @Test
+ public void testRemoveFilesWithLiveTservers() throws IOException {
+ GCStatus status = new GCStatus();
+ GarbageCollectWriteAheadLogs realGCWAL =
getGCWALForRemoveFileTest(status, true);
+ String server = "tserver1+9997";
+ Path p1 = new Path("hdfs://localhost:9000/accumulo/wal/" + server +
"/" + UUID.randomUUID().toString());
+ Map<String,ArrayList<Path>> serverToFileMap =
getServerToFileMap1(server, p1);
+
+ realGCWAL.removeFiles(getEmptyMap(), serverToFileMap, getEmptyMap(),
status);
+
+ MethodCalls calls = ((GCWALPartialMock) realGCWAL).methodCalls;
+ assertEquals("Only one method should have been called", 1,
calls.size());
+ assertEquals("Method should be askTserverToRemoveWAL",
"askTserverToRemoveWAL", calls.getFirstEntryMethod());
+ assertEquals("First param should be address",
HostAndPort.fromString(server.replaceAll("[+]", ":")),
calls.getFirstEntryArg(0));
+ assertTrue("Second param should be an AccumuloConfiguration",
calls.getFirstEntryArg(1) instanceof AccumuloConfiguration);
+ Entry<String,ArrayList<Path>> firstServerToFileMap =
serverToFileMap.entrySet().iterator().next();
+ assertEquals("Third param should be the entry", firstServerToFileMap,
calls.getFirstEntryArg(2));
+ assertEquals("Forth param should be the status", status,
calls.getFirstEntryArg(3));
+ }
+
+ @Test
+ public void testRemoveFilesRemovesSortedWALs() throws IOException {
+ GCStatus status = new GCStatus();
+ GarbageCollectWriteAheadLogs realGCWAL =
getGCWALForRemoveFileTest(status, true);
+ Map<String,ArrayList<Path>> serverToFileMap = new
HashMap<String,ArrayList<Path>>();
+ Map<String,Path> sortedWALogs = new HashMap<String,Path>();
+ Path p1 = new Path("hdfs://localhost:9000/accumulo/wal/tserver1+9997/"
+ UUID.randomUUID().toString());
+ sortedWALogs.put("junk", p1); // TODO: see if this key is actually
used here, maybe can be removed
+
+ realGCWAL.removeFiles(getEmptyMap(), serverToFileMap, sortedWALogs,
status);
+ MethodCalls calls = ((GCWALPartialMock) realGCWAL).methodCalls;
+ assertEquals("Only one method should have been called", 1,
calls.size());
+ assertEquals("Method should be removeSortedWAL", "removeSortedWAL",
calls.getFirstEntryMethod());
+ assertEquals("First param should be the Path", p1,
calls.getFirstEntryArg(0));
+
+ }
+
+ static String GCWAL_DEAD_DIR = "gcwal-collect-deadtserver";
+ static String GCWAL_DEAD_TSERVER = "tserver1";
+ static String GCWAL_DEAD_TSERVER_PORT = "9995";
+ static String GCWAL_DEAD_TSERVER_COLLECT_FILE =
UUID.randomUUID().toString();
+
+ class GCWALDeadTserverCollectMock extends GarbageCollectWriteAheadLogs {
+
+ public GCWALDeadTserverCollectMock(Instance i, VolumeManager vm,
boolean useTrash) throws IOException {
+ super(i, vm, useTrash);
+ }
+
+ @Override
+ boolean holdsLock(HostAndPort addr) {
+ // tries use zookeeper
+ return false;
+ }
+
+ @Override
+ Map<String,Path> getSortedWALogs() {
+ return new HashMap<String,Path>();
+ }
+
+ @Override
+ int scanServers(Map<Path,String> fileToServerMap, Map<String,Path>
nameToFileMap) throws Exception {
+ String sep = File.separator;
+ Path p = new Path(System.getProperty("java.io.tmpdir") + sep +
GCWAL_DEAD_DIR + sep + GCWAL_DEAD_TSERVER + "+" + GCWAL_DEAD_TSERVER_PORT + sep
+ + GCWAL_DEAD_TSERVER_COLLECT_FILE);
+ fileToServerMap.put(p, GCWAL_DEAD_TSERVER + ":" +
GCWAL_DEAD_TSERVER_PORT);
+ nameToFileMap.put(GCWAL_DEAD_TSERVER_COLLECT_FILE, p);
+ return 1;
+ }
+
+ @Override
+ int removeMetadataEntries(Map<String,Path> nameToFileMap,
Map<String,Path> sortedWALogs, GCStatus status) throws IOException,
KeeperException,
+ InterruptedException {
+ return 0;
+ }
+
+ long getGCWALDeadServerWaitTime(AccumuloConfiguration conf) {
+ // tries to use zookeeper
+ return 1000l;
+ }
+ }
+
+ @Test
+ public void testCollectWithDeadTserver() throws IOException,
InterruptedException {
+ Instance i = new MockInstance();
+ File walDir = new File(System.getProperty("java.io.tmpdir") +
File.separator + GCWAL_DEAD_DIR);
+ File walFileDir = new File(walDir + File.separator +
GCWAL_DEAD_TSERVER + "+" + GCWAL_DEAD_TSERVER_PORT);
+ File walFile = new File(walFileDir + File.separator +
GCWAL_DEAD_TSERVER_COLLECT_FILE);
+ if (!walFileDir.exists()) {
+ walFileDir.mkdirs();
+ new FileOutputStream(walFile).close();
+ }
+
+ try {
+ VolumeManager vm = VolumeManagerImpl.getLocal(walDir.toString());
+ GarbageCollectWriteAheadLogs gcwal2 = new
GCWALDeadTserverCollectMock(i, vm, false);
+ GCStatus status = new GCStatus(new GcCycleStats(), new
GcCycleStats(), new GcCycleStats(), new GcCycleStats());
+
+ gcwal2.collect(status);
+
+ assertTrue("File should not be deleted", walFile.exists());
+ assertEquals("Should have one candidate", 1,
status.lastLog.getCandidates());
+ assertEquals("Should not have deleted that file", 0,
status.lastLog.getDeleted());
+
+ sleep(2000);
+ gcwal2.collect(status);
--- End diff --
nice test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
