You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2018/02/14 21:17:37 UTC
[accumulo] branch 1.7 updated: ACCUMULO-4809 Avoid blocking during
session cleanup (#383)
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 <ke...@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.