accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] branch 1.9 updated: fixes #538 fix WAL recovery code (#541)
Date Fri, 29 Jun 2018 16:50:40 GMT
This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch 1.9
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/1.9 by this push:
     new 7e3d975  fixes #538 fix WAL recovery code (#541)
7e3d975 is described below

commit 7e3d975026edb9840ca42c7288a1ff8faa3bd106
Author: Keith Turner <keith@deenlo.com>
AuthorDate: Fri Jun 29 12:50:38 2018 -0400

    fixes #538 fix WAL recovery code (#541)
    
    There are two changes in this patch. First, removed a sanity check from the code
    that resulted in false positives.  Second, changed recovery code to use last
    compaction finish event for recovery seq #. Code no longer looks for last start
    to finish, because there are valid situations where there is no start event.
---
 .../accumulo/tserver/log/SortedLogRecovery.java    | 60 ++++++++++------------
 .../tserver/log/SortedLogRecoveryTest.java         | 48 ++++++++---------
 2 files changed, 48 insertions(+), 60 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java
index c0fa3a2..642064f 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java
@@ -160,13 +160,14 @@ public class SortedLogRecovery {
 
   }
 
-  private long findLastStartToFinish(List<Path> recoveryLogs, Set<String> tabletFiles,
int tabletId)
+  private long findRecoverySeq(List<Path> recoveryLogs, Set<String> tabletFiles,
int tabletId)
       throws IOException {
     HashSet<String> suffixes = new HashSet<>();
     for (String path : tabletFiles)
       suffixes.add(getPathSuffix(path));
 
     long lastStart = 0;
+    long lastFinish = 0;
     long recoverySeq = 0;
 
     try (RecoveryLogsIterator rli = new RecoveryLogsIterator(fs, recoveryLogs,
@@ -176,52 +177,43 @@ public class SortedLogRecovery {
 
       String lastStartFile = null;
       LogEvents lastEvent = null;
-      boolean firstEventWasFinish = false;
-      boolean sawStartFinish = false;
 
       while (ddi.hasNext()) {
         LogFileKey key = ddi.next().getKey();
 
         checkState(key.seq >= 0, "Unexpected negative seq %s for tabletId %s", key.seq,
tabletId);
         checkState(key.tabletId == tabletId); // should only fail if bug elsewhere
-
-        if (key.event == COMPACTION_START) {
-          checkState(key.seq >= lastStart); // should only fail if bug elsewhere
-          lastStart = key.seq;
-          lastStartFile = key.filename;
-        } else if (key.event == COMPACTION_FINISH) {
-          if (lastEvent == null) {
-            firstEventWasFinish = true;
-          } else if (lastEvent == COMPACTION_FINISH) {
-            throw new IllegalStateException(
-                "Saw consecutive COMPACTION_FINISH events " + key.tabletId + " " + key.seq);
-          } else {
-            if (key.seq <= lastStart) {
-              throw new IllegalStateException(
-                  "Compaction finish <= start " + lastStart + " " + key.seq);
-            }
-            recoverySeq = lastStart;
-            lastStartFile = null;
-            sawStartFinish = true;
-          }
-        } else {
-          throw new IllegalStateException("Non compaction event seen " + key.event);
+        checkState(key.seq >= Math.max(lastFinish, lastStart)); // should only fail if
bug elsewhere
+
+        switch (key.event) {
+          case COMPACTION_START:
+            lastStart = key.seq;
+            lastStartFile = key.filename;
+            break;
+          case COMPACTION_FINISH:
+            checkState(key.seq > lastStart, "Compaction finish <= start %s %s %s",
key.tabletId,
+                key.seq, lastStart);
+            checkState(lastEvent != COMPACTION_FINISH,
+                "Saw consecutive COMPACTION_FINISH events %s %s %s", key.tabletId, lastFinish,
+                key.seq);
+            lastFinish = key.seq;
+            break;
+          default:
+            throw new IllegalStateException("Non compaction event seen " + key.event);
         }
 
         lastEvent = key.event;
       }
 
-      if (firstEventWasFinish && !sawStartFinish) {
-        throw new IllegalStateException("COMPACTION_FINISH (without preceding COMPACTION_START)"
-            + " is not followed by a successful minor compaction.");
-      }
-
-      if (lastStartFile != null && suffixes.contains(getPathSuffix(lastStartFile)))
{
-        // There was no compaction finish event, however the last compaction start event
has a file
-        // in the metadata table, so the compaction finished.
+      if (lastEvent == COMPACTION_START && suffixes.contains(getPathSuffix(lastStartFile)))
{
+        // There was no compaction finish event following this start, however the last compaction
+        // start event has a file in the metadata table, so the compaction finished.
         log.debug("Considering compaction start {} {} finished because file {} in metadata
table",
             tabletId, lastStart, getPathSuffix(lastStartFile));
         recoverySeq = lastStart;
+      } else {
+        // Recover everything >= the maximum finish sequence number if its set, otherwise
return 0.
+        recoverySeq = Math.max(0, lastFinish - 1);
       }
     }
     return recoverySeq;
@@ -277,7 +269,7 @@ public class SortedLogRecovery {
     }
 
     // Find the seq # for the last compaction that started and finished
-    long recoverySeq = findLastStartToFinish(recoveryLogs, tabletFiles, tabletId);
+    long recoverySeq = findRecoverySeq(recoveryLogs, tabletFiles, tabletId);
 
     log.info("Recovering mutations, tablet:{} tabletId:{} seq:{} logs:{}", extent, tabletId,
         recoverySeq, asNames(recoveryLogs));
diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java
b/server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java
index bdf5210..53d6851 100644
--- a/server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java
+++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java
@@ -395,7 +395,7 @@ public class SortedLogRecoveryTest {
         createKeyValue(COMPACTION_START, 3, 1, "/t1/f1"), createKeyValue(MUTATION, 2, 1,
ignored),
         createKeyValue(MUTATION, 4, 1, m),};
     KeyValue entries2[] = new KeyValue[] {createKeyValue(OPEN, 5, -1, "1"),
-        createKeyValue(DEFINE_TABLET, 6, 1, extent), createKeyValue(COMPACTION_FINISH, 7,
1, null),
+        createKeyValue(DEFINE_TABLET, 4, 1, extent), createKeyValue(COMPACTION_FINISH, 4,
1, null),
         createKeyValue(MUTATION, 8, 1, m2),};
     Map<String,KeyValue[]> logs = new TreeMap<>();
     logs.put("entries", entries);
@@ -420,8 +420,8 @@ public class SortedLogRecoveryTest {
     KeyValue entries[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
         createKeyValue(DEFINE_TABLET, 1, 1, extent), createKeyValue(COMPACTION_FINISH, 2,
1, null),
         createKeyValue(COMPACTION_START, 4, 1, "/t1/f1"),
-        createKeyValue(COMPACTION_FINISH, 6, 1, null), createKeyValue(MUTATION, 3, 1, ignored),
-        createKeyValue(MUTATION, 5, 1, m), createKeyValue(MUTATION, 7, 1, m2),};
+        createKeyValue(COMPACTION_FINISH, 5, 1, null), createKeyValue(MUTATION, 3, 1, ignored),
+        createKeyValue(MUTATION, 5, 1, m), createKeyValue(MUTATION, 5, 1, m2),};
     Map<String,KeyValue[]> logs = new TreeMap<>();
     logs.put("entries", entries);
     // Recover
@@ -448,10 +448,10 @@ public class SortedLogRecoveryTest {
         createKeyValue(COMPACTION_START, 3, 1, "/t1/f1"), createKeyValue(MUTATION, 2, 1,
ignored),
         createKeyValue(MUTATION, 4, 1, m),};
     KeyValue entries2[] = new KeyValue[] {createKeyValue(OPEN, 5, -1, "1"),
-        createKeyValue(DEFINE_TABLET, 6, 1, extent), createKeyValue(MUTATION, 7, 1, m2),};
+        createKeyValue(DEFINE_TABLET, 4, 1, extent), createKeyValue(MUTATION, 4, 1, m2),};
     KeyValue entries3[] = new KeyValue[] {createKeyValue(OPEN, 8, -1, "1"),
-        createKeyValue(DEFINE_TABLET, 9, 1, extent), createKeyValue(COMPACTION_FINISH, 10,
1, null),
-        createKeyValue(MUTATION, 11, 1, m3),};
+        createKeyValue(DEFINE_TABLET, 4, 1, extent), createKeyValue(COMPACTION_FINISH, 4,
1, null),
+        createKeyValue(MUTATION, 4, 1, m3),};
     Map<String,KeyValue[]> logs = new TreeMap<>();
     logs.put("entries", entries);
     logs.put("entries2", entries2);
@@ -460,9 +460,9 @@ public class SortedLogRecoveryTest {
     List<Mutation> mutations = recover(logs, extent);
     // Verify recovered data
     Assert.assertEquals(3, mutations.size());
-    Assert.assertEquals(m, mutations.get(0));
-    Assert.assertEquals(m2, mutations.get(1));
-    Assert.assertEquals(m3, mutations.get(2));
+    Assert.assertTrue(mutations.contains(m));
+    Assert.assertTrue(mutations.contains(m2));
+    Assert.assertTrue(mutations.contains(m3));
   }
 
   @Test
@@ -498,8 +498,8 @@ public class SortedLogRecoveryTest {
     m3.put(cf, cq, value);
     KeyValue entries[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
         createKeyValue(DEFINE_TABLET, 1, 1, extent),
-        createKeyValue(COMPACTION_START, 2, 1, "/t1/f1"),
-        createKeyValue(COMPACTION_FINISH, 4, 1, null), createKeyValue(MUTATION, 3, 1, m),};
+        createKeyValue(COMPACTION_START, 3, 1, "/t1/f1"),
+        createKeyValue(COMPACTION_FINISH, 4, 1, null), createKeyValue(MUTATION, 4, 1, m),};
     KeyValue entries2[] = new KeyValue[] {createKeyValue(OPEN, 5, -1, "1"),
         createKeyValue(DEFINE_TABLET, 6, 1, extent),
         createKeyValue(COMPACTION_START, 8, 1, "/t1/f1"), createKeyValue(MUTATION, 7, 1,
m2),
@@ -620,8 +620,8 @@ public class SortedLogRecoveryTest {
 
     KeyValue entries[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
         createKeyValue(DEFINE_TABLET, 1, 1, extent), createKeyValue(DEFINE_TABLET, 1, 2,
extent),
-        createKeyValue(MUTATION, 2, 2, ignored), createKeyValue(COMPACTION_START, 3, 2, "/t1/f1"),
-        createKeyValue(MUTATION, 4, 2, m), createKeyValue(COMPACTION_FINISH, 6, 2, null),};
+        createKeyValue(MUTATION, 2, 2, ignored), createKeyValue(COMPACTION_START, 5, 2, "/t1/f1"),
+        createKeyValue(MUTATION, 6, 2, m), createKeyValue(COMPACTION_FINISH, 6, 2, null),};
 
     Arrays.sort(entries);
 
@@ -730,7 +730,7 @@ public class SortedLogRecoveryTest {
     KeyValue entries1[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
         createKeyValue(DEFINE_TABLET, 7, 10, e1), createKeyValue(DEFINE_TABLET, 5, 11, e2),
         createKeyValue(MUTATION, 8, 10, m1), createKeyValue(COMPACTION_START, 9, 10, "/t/f1"),
-        createKeyValue(MUTATION, 10, 10, m2), createKeyValue(COMPACTION_FINISH, 11, 10, null),
+        createKeyValue(MUTATION, 10, 10, m2), createKeyValue(COMPACTION_FINISH, 10, 10, null),
         createKeyValue(MUTATION, 6, 11, m3), createKeyValue(COMPACTION_START, 7, 11, "/t/f2"),
         createKeyValue(MUTATION, 8, 11, m4)};
 
@@ -749,7 +749,7 @@ public class SortedLogRecoveryTest {
     Assert.assertEquals(m4, mutations2.get(1));
 
     KeyValue entries2[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
-        createKeyValue(DEFINE_TABLET, 9, 11, e2), createKeyValue(COMPACTION_FINISH, 10, 11,
null)};
+        createKeyValue(DEFINE_TABLET, 9, 11, e2), createKeyValue(COMPACTION_FINISH, 8, 11,
null)};
     Arrays.sort(entries2);
     logs.put("entries2", entries2);
 
@@ -823,25 +823,21 @@ public class SortedLogRecoveryTest {
     Mutation m1 = new ServerMutation(new Text("r1"));
     m1.put("f1", "q1", "v1");
 
-    Mutation m2 = new ServerMutation(new Text("r2"));
-    m2.put("f1", "q1", "v2");
-
     // The presence of only a compaction finish event indicates the write ahead logs are
incomplete
     // in some way. This should cause an exception.
 
     KeyValue entries1[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
-        createKeyValue(DEFINE_TABLET, 100, 10, extent), createKeyValue(MUTATION, 100, 10,
m1),
-        createKeyValue(COMPACTION_FINISH, 102, 10, null), createKeyValue(MUTATION, 105, 10,
m2)};
+        createKeyValue(DEFINE_TABLET, 100, 10, extent),
+        createKeyValue(COMPACTION_FINISH, 102, 10, null), createKeyValue(MUTATION, 102, 10,
m1)};
 
     Arrays.sort(entries1);
 
     Map<String,KeyValue[]> logs = new TreeMap<>();
     logs.put("entries1", entries1);
 
-    thrown.expect(IllegalStateException.class);
-    thrown.expectMessage(
-        LogEvents.COMPACTION_FINISH.name() + " (without preceding " + LogEvents.COMPACTION_START);
-    recover(logs, extent);
+    List<Mutation> mutations = recover(logs, extent);
+    Assert.assertEquals(1, mutations.size());
+    Assert.assertEquals(m1, mutations.get(0));
   }
 
   @Test
@@ -884,8 +880,8 @@ public class SortedLogRecoveryTest {
     KeyValue entries1[] = new KeyValue[] {createKeyValue(OPEN, 0, -1, "1"),
         createKeyValue(DEFINE_TABLET, 100, 10, extent), createKeyValue(MUTATION, 100, 10,
m1),
         createKeyValue(COMPACTION_START, 102, 10, "/t/f1"),
-        createKeyValue(COMPACTION_FINISH, 104, 10, null),
-        createKeyValue(COMPACTION_FINISH, 104, 10, null), createKeyValue(MUTATION, 103, 10,
m2)};
+        createKeyValue(COMPACTION_FINISH, 103, 10, null),
+        createKeyValue(COMPACTION_FINISH, 103, 10, null), createKeyValue(MUTATION, 103, 10,
m2)};
 
     Arrays.sort(entries1);
 


Mime
View raw message