accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] branch 1.7 updated: ACCUMULO-4809 Avoid blocking during session cleanup (#383)
Date Wed, 14 Feb 2018 21:17:37 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/1.7 by this push:
     new f40f7c1  ACCUMULO-4809 Avoid blocking during session cleanup (#383)
f40f7c1 is described below

commit f40f7c1ce1d27856d57193fab516c39c85d22d52
Author: Keith Turner <keith@deenlo.com>
AuthorDate: Wed Feb 14 16:17:34 2018 -0500

    ACCUMULO-4809 Avoid blocking during session cleanup (#383)
---
 .../accumulo/tserver/session/SessionManager.java   | 78 +++++++++++-----------
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java
index 78c351d..a03c032 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/session/SessionManager.java
@@ -169,36 +169,38 @@ public class SessionManager {
   }
 
   private void sweep(final long maxIdle, final long maxUpdateIdle) {
-    List<Session> sessionsToCleanup = new ArrayList<>();
-    synchronized (this) {
-      Iterator<Session> iter = sessions.values().iterator();
-      while (iter.hasNext()) {
-        Session session = iter.next();
-        long configuredIdle = maxIdle;
-        if (session instanceof UpdateSession) {
-          configuredIdle = maxUpdateIdle;
-        }
-        long idleTime = System.currentTimeMillis() - session.lastAccessTime;
-        if (idleTime > configuredIdle && !session.reserved) {
-          log.info("Closing idle session from user=" + session.getUser() + ", client=" +
session.client + ", idle=" + idleTime + "ms");
-          iter.remove();
-          sessionsToCleanup.add(session);
+    // In Accumulo's current code only one thread will ever call this method. However if
multiple threads called this method concurrently it could result in
+    // sessions being lost. This method synchronizes on idleSessions to prevent the loss.
Its not expected that anything else will synchronize on idleSessions.
+    synchronized (idleSessions) {
+      synchronized (this) {
+        Iterator<Session> iter = sessions.values().iterator();
+        while (iter.hasNext()) {
+          Session session = iter.next();
+          long configuredIdle = maxIdle;
+          if (session instanceof UpdateSession) {
+            configuredIdle = maxUpdateIdle;
+          }
+          long idleTime = System.currentTimeMillis() - session.lastAccessTime;
+          if (idleTime > configuredIdle && !session.reserved) {
+            log.info("Closing idle session from user=" + session.getUser() + ", client="
+ session.client + ", idle=" + idleTime + "ms");
+            iter.remove();
+            idleSessions.add(session);
+          }
         }
       }
-    }
-
-    // do clean up outside of lock for TabletServer in a synchronized block for simplicity
vice a synchronized list
 
-    synchronized (idleSessions) {
+      List<Session> sessionsToCleanup = new ArrayList<>();
 
-      sessionsToCleanup.addAll(idleSessions);
-
-      idleSessions.clear();
+      // do clean up outside of lock for TabletServer
+      for (Session session : idleSessions) {
+        if (!session.cleanup()) {
+          sessionsToCleanup.add(session);
+        }
+      }
 
-      // perform cleanup for all of the sessions
-      for (Session session : sessionsToCleanup) {
-        if (!session.cleanup())
-          idleSessions.add(session);
+      synchronized (this) {
+        idleSessions.clear();
+        idleSessions.addAll(sessionsToCleanup);
       }
     }
   }
@@ -234,16 +236,14 @@ public class SessionManager {
     Map<String,MapCounter<ScanRunState>> counts = new HashMap<>();
     Set<Entry<Long,Session>> copiedIdleSessions = new HashSet<>();
 
-    synchronized (idleSessions) {
-      /**
-       * Add sessions so that get the list returned in the active scans call
-       */
-      for (Session session : idleSessions) {
-        copiedIdleSessions.add(Maps.immutableEntry(expiredSessionMarker, session));
-      }
+    /**
+     * Add sessions so that get the list returned in the active scans call
+     */
+    for (Session session : idleSessions) {
+      copiedIdleSessions.add(Maps.immutableEntry(expiredSessionMarker, session));
     }
 
-    for (Entry<Long,Session> entry : sessions.entrySet()) {
+    for (Entry<Long,Session> entry : Iterables.concat(sessions.entrySet(), copiedIdleSessions))
{
 
       Session session = entry.getValue();
       @SuppressWarnings("rawtypes")
@@ -286,13 +286,11 @@ public class SessionManager {
     final long ct = System.currentTimeMillis();
     final Set<Entry<Long,Session>> copiedIdleSessions = new HashSet<>();
 
-    synchronized (idleSessions) {
-      /**
-       * Add sessions so that get the list returned in the active scans call
-       */
-      for (Session session : idleSessions) {
-        copiedIdleSessions.add(Maps.immutableEntry(expiredSessionMarker, session));
-      }
+    /**
+     * Add sessions so that get the list returned in the active scans call
+     */
+    for (Session session : idleSessions) {
+      copiedIdleSessions.add(Maps.immutableEntry(expiredSessionMarker, session));
     }
 
     for (Entry<Long,Session> entry : Iterables.concat(sessions.entrySet(), copiedIdleSessions))
{

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

Mime
View raw message