[accumulo] 04/04: fixes #477 Stop putting -1 in WAL (#458)

[kt...@apache.org]

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

commit b8f574fc282cdc7fc4f92b7bf0d5db49bc7ee4ce
Author: Keith Turner <kt...@apache.org>
AuthorDate: Mon May 7 16:11:13 2018 -0400

    fixes #477 Stop putting -1 in WAL (#458)
---
 .../main/java/org/apache/accumulo/tserver/TabletServer.java    | 10 +++++-----
 .../org/apache/accumulo/tserver/log/SortedLogRecovery.java     |  5 ++++-
 .../main/java/org/apache/accumulo/tserver/tablet/Tablet.java   |  2 +-
 .../java/org/apache/accumulo/tserver/log/LogFileKeyTest.java   |  2 ++
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index a0c2a81..f87841d 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -3276,12 +3276,12 @@ public class TabletServer extends AccumuloServerContext implements Runnable {
     logger.recover(fs, extent, tconf, recoveryLogs, tabletFiles, mutationReceiver);
   }
 
-  public int createLogId(KeyExtent tablet) {
-    AccumuloConfiguration acuTableConf = getTableConfiguration(tablet);
-    if (DurabilityImpl.fromString(acuTableConf.get(Property.TABLE_DURABILITY)) != Durability.NONE) {
-      return logIdGenerator.incrementAndGet();
+  public int createLogId() {
+    int logId = logIdGenerator.incrementAndGet();
+    if (logId < 0) {
+      throw new IllegalStateException("Log Id rolled");
     }
-    return -1;
+    return logId;
   }
 
   public TableConfiguration getTableConfiguration(KeyExtent extent) {
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 5692a81..ebfa92f 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
@@ -78,7 +78,10 @@ public class SortedLogRecovery {
   static LogFileKey minKey(LogEvents event) {
     LogFileKey key = new LogFileKey();
     key.event = event;
-    key.tabletId = 0;
+    // see GitHub issue #477. There was a bug that caused -1 to end up in tabletId. If this happens
+    // want to detect it and fail since recovery is dubious in this situation . Other code should
+    // fail if the id is actually -1 in data.
+    key.tabletId = -1;
     key.seq = 0;
     return key;
   }
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
index 18d8daf..1e0ffcc 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
@@ -330,7 +330,7 @@ public class Tablet implements TabletCommitter {
     this.splitCreationTime = data.getSplitTime();
     this.tabletTime = TabletTime.getInstance(data.getTime());
     this.persistedTime = tabletTime.getTime();
-    this.logId = tabletServer.createLogId(extent);
+    this.logId = tabletServer.createLogId();
 
     TableConfiguration tblConf = tabletServer.getTableConfiguration(extent);
     if (null == tblConf) {
diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/log/LogFileKeyTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/log/LogFileKeyTest.java
index 0aacef2..8965094 100644
--- a/server/tserver/src/test/java/org/apache/accumulo/tserver/log/LogFileKeyTest.java
+++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/log/LogFileKeyTest.java
@@ -57,6 +57,8 @@ public class LogFileKeyTest {
     // add keys in expected sort order
     keys.add(nk(LogEvents.OPEN, 0, 0));
 
+    // there was a bug that was putting -1 in WAL for tabletId, ensure this data sorts as expected
+    keys.add(nk(LogEvents.DEFINE_TABLET, -1, 0));
     keys.add(nk(LogEvents.DEFINE_TABLET, 3, 6));
     keys.add(nk(LogEvents.DEFINE_TABLET, 3, 7));
     keys.add(nk(LogEvents.DEFINE_TABLET, 4, 2));

-- 
To stop receiving notification emails like this one, please contact
kturner@apache.org.