You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2018/06/29 16:50:40 UTC
[GitHub] keith-turner closed pull request #541: fixes #538 fix WAL recovery
code
keith-turner closed pull request #541: fixes #538 fix WAL recovery code
URL: https://github.com/apache/accumulo/pull/541
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 c0fa3a245b..642064f0fd 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 void remove() {
}
- 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 @@ private long findLastStartToFinish(List<Path> recoveryLogs, Set<String> tabletFi
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 void recover(KeyExtent extent, List<Path> recoveryLogs, Set<String> table
}
// 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 bdf5210564..53d685197d 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 void testGetMutationsAfterCompactionStart() throws IOException {
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 void testDoubleFinish() throws IOException {
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 void testCompactionCrossesLogs2() throws IOException {
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 void testCompactionCrossesLogs2() throws IOException {
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 void testBug2() throws IOException {
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 void testMultipleTabletDefinition() throws Exception {
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 void testMultipleTablets() throws IOException {
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 void testMultipleTablets() throws IOException {
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 void testOnlyCompactionFinishEvent() throws IOException {
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 void testDuplicateCompactionFinishEvents() throws IOException {
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);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services