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: Fix race condition when getting WALs for dead tserver (#866)
Date Wed, 02 Jan 2019 19:16:37 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 2c2840b  Fix race condition when getting WALs for dead tserver (#866)
2c2840b is described below

commit 2c2840b1873282354fffad2d76774fd31cc2fd41
Author: Keith Turner <kturner@apache.org>
AuthorDate: Wed Jan 2 14:16:33 2019 -0500

    Fix race condition when getting WALs for dead tserver (#866)
    
    When a tablet server dies the master gets its WALs from ZK.  In ZK there
    is a list of WALs per tserver. Each WAL in ZK has state that is either
    OPEN, CLOSED, or UNREFERENCED.  The master needs a list of OPEN and
    CLOSED logs for dead tservers.  While the master is trying to obtain
    this list its possible that the Accumulo GC may delete an UNREFERENCED
    WAL.  If this happened then the code before this commit would return an
    empty list of WALs. This could result in OPEN and CLOSED logs being
    ignored for recovery which could result in data loss.  This patch fixes
    the race condition.
    
    This bug was observed while looking into an exception I noticed while
    writing test for #860.  In the IT I was frequently calling another
    WalStateManager function and noticed NoNodeExceptions.  As a result I
    examined how the entire class handled NoNode race conditions and found
    this bug.  I noticed some other possible NoNode race condition, but do
    not think this these would occur with the current way the code is called.
---
 .../accumulo/server/log/WalStateManager.java       | 24 +++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/server/base/src/main/java/org/apache/accumulo/server/log/WalStateManager.java
b/server/base/src/main/java/org/apache/accumulo/server/log/WalStateManager.java
index 22e9ee1..12f3133 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/log/WalStateManager.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/log/WalStateManager.java
@@ -163,9 +163,22 @@ public class WalStateManager {
       String zpath = root() + "/" + tsi.toString();
       zoo.sync(zpath);
       for (String child : zoo.getChildren(zpath)) {
-        Pair<WalState,Path> parts = parse(zoo.getData(zpath + "/" + child, null));
-        if (parts.getFirst() != WalState.UNREFERENCED) {
-          result.add(parts.getSecond());
+        byte[] zdata = null;
+        try {
+          // This function is called by the Master. Its possible that Accumulo GC deletes
an
+          // unreferenced WAL in ZK after the call to getChildren above. Catch this exception
inside
+          // the loop so that not all children are ignored.
+          zdata = zoo.getData(zpath + "/" + child, null);
+        } catch (KeeperException.NoNodeException e) {
+          log.debug("WAL state removed {} {} during getWalsInUse.  Likely a race condition
between "
+              + "master and GC.", tsi, child);
+        }
+
+        if (zdata != null) {
+          Pair<WalState,Path> parts = parse(zdata);
+          if (parts.getFirst() != WalState.UNREFERENCED) {
+            result.add(parts.getSecond());
+          }
         }
       }
     } catch (KeeperException.NoNodeException e) {
@@ -187,6 +200,9 @@ public class WalStateManager {
         if (logs == null) {
           result.put(inst, logs = new ArrayList<>());
         }
+
+        // This function is called by the Accumulo GC which deletes WAL markers. Therefore
we do not
+        // expect the following call to fail because the WAL info in ZK was deleted.
         for (String idString : zoo.getChildren(path + "/" + child)) {
           logs.add(UUID.fromString(idString));
         }
@@ -212,6 +228,8 @@ public class WalStateManager {
     Map<Path,WalState> result = new HashMap<>();
     for (Entry<TServerInstance,List<UUID>> entry : getAllMarkers().entrySet())
{
       for (UUID id : entry.getValue()) {
+        // This function is called by the Accumulo GC which deletes WAL markers. Therefore
we do not
+        // expect the following call to fail because the WAL info in ZK was deleted.
         Pair<WalState,Path> state = state(entry.getKey(), id);
         result.put(state.getSecond(), state.getFirst());
       }


Mime
View raw message