details: https://code.openbravo.com/erp/devel/pi/rev/5aed2a398dac changeset: 32412:5aed2a398dac user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Wed Jul 05 14:47:20 2017 +0200 summary: related to bug 36419: no request processed while killing abandoned sessions
Deactivate abandoned sessions in a separate DB transaction, in this manner the time locks in ad_session are held is reduced to only this action, before those locks were held till the thread was terminated. details: https://code.openbravo.com/erp/devel/pi/rev/76e0f1c4b33e changeset: 32413:76e0f1c4b33e user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Thu Jul 06 07:41:00 2017 +0200 summary: fixed bug 36419: no request processed while killing abandoned sessions * ActivationKey.getInstance is not synchronized anymore * Synchronization is now reduced to the places changing state * Session deactivation monitored by a lock to ensure there is at most one thread in this block, in this way, at most one thread can be wating for DB lock at this position details: https://code.openbravo.com/erp/devel/pi/rev/79ddd2858936 changeset: 32414:79ddd2858936 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Thu Jul 06 08:11:02 2017 +0200 summary: related to bug 36419: no request processed while killing abandoned sessions Removed synchronization in WS call counter details: https://code.openbravo.com/erp/devel/pi/rev/0d877e9b8212 changeset: 32415:0d877e9b8212 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Thu Jul 06 08:21:45 2017 +0200 summary: related to bug 36419: no request processed while killing abandoned sessions Log stack traces in case of error details: https://code.openbravo.com/erp/devel/pi/rev/8b2480998f02 changeset: 32416:8b2480998f02 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Thu Jul 06 10:02:38 2017 +0200 summary: fixed bug 36425: mobile sessions can be kicked out in cluster Don't try to invalidate those sessions not present in current node. details: https://code.openbravo.com/erp/devel/pi/rev/c557445d5d66 changeset: 32417:c557445d5d66 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Thu Jul 06 14:07:15 2017 +0200 summary: fixed bug 36434: unlikely argument type warnings diffstat: src-core/src/org/openbravo/base/VariablesBase.java | 2 +- src/org/openbravo/dal/xml/EntityResolver.java | 4 +- src/org/openbravo/erpCommon/obps/ActivationKey.java | 211 +++++++++----- src/org/openbravo/erpCommon/obps/ActivationKey_data.xsql | 36 ++ src/org/openbravo/service/web/UserContextCache.java | 8 +- 5 files changed, 175 insertions(+), 86 deletions(-) diffs (truncated from 439 to 300 lines): diff -r 01727c5b58ac -r c557445d5d66 src-core/src/org/openbravo/base/VariablesBase.java --- a/src-core/src/org/openbravo/base/VariablesBase.java Wed Jul 05 14:16:09 2017 +0200 +++ b/src-core/src/org/openbravo/base/VariablesBase.java Thu Jul 06 14:07:15 2017 +0200 @@ -1043,7 +1043,7 @@ } } - if (auxStr == null || auxStr.length == 0 || auxStr.equals("")) { + if (auxStr == null || auxStr.length == 0) { if (required) { throw new ServletException("Request IN parameter required: " + parameter); } else { diff -r 01727c5b58ac -r c557445d5d66 src/org/openbravo/dal/xml/EntityResolver.java --- a/src/org/openbravo/dal/xml/EntityResolver.java Wed Jul 05 14:16:09 2017 +0200 +++ b/src/org/openbravo/dal/xml/EntityResolver.java Thu Jul 06 14:07:15 2017 +0200 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2008-2011 Openbravo SLU + * All portions are Copyright (C) 2008-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -360,7 +360,7 @@ List<RefDataLoaded> refLoadeds) { BaseOBObject result = null; for (RefDataLoaded refLoaded : refLoadeds) { - if (refLoaded.getClientId().equals('0') && refLoaded.getOrgId().equals("0")) { + if (refLoaded.getClientId().equals("0") && refLoaded.getOrgId().equals("0")) { result = doSearch(refLoaded.getSpecificId(), entity, "0", "0"); if (result != null) { return result; diff -r 01727c5b58ac -r c557445d5d66 src/org/openbravo/erpCommon/obps/ActivationKey.java --- a/src/org/openbravo/erpCommon/obps/ActivationKey.java Wed Jul 05 14:16:09 2017 +0200 +++ b/src/org/openbravo/erpCommon/obps/ActivationKey.java Thu Jul 06 14:07:15 2017 +0200 @@ -35,6 +35,8 @@ import java.security.PublicKey; import java.security.Signature; import java.security.spec.X509EncodedKeySpec; +import java.sql.Connection; +import java.sql.SQLException; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -47,11 +49,15 @@ import java.util.Map; import java.util.Properties; import java.util.UUID; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.zip.CRC32; import javax.crypto.Cipher; import javax.enterprise.inject.spi.Bean; import javax.enterprise.inject.spi.BeanManager; +import javax.servlet.ServletException; import javax.servlet.http.HttpSession; import org.apache.commons.io.FileUtils; @@ -82,6 +88,7 @@ import org.openbravo.erpCommon.utility.OBMessageUtils; import org.openbravo.erpCommon.utility.SystemInfo; import org.openbravo.erpCommon.utility.Utility; +import org.openbravo.exception.NoConnectionAvailableException; import org.openbravo.model.ad.access.Session; import org.openbravo.model.ad.module.Module; import org.openbravo.model.ad.system.HeartbeatLog; @@ -133,7 +140,7 @@ private boolean inconsistentInstance = false; private long maxWsCalls; - private long wsDayCounter; + private AtomicLong wsDayCounter; private Date initWsCountTime; private List<Date> exceededInLastDays; @@ -144,6 +151,9 @@ private static final String GOLDEN_EXCLUDED = "GOLDENEXCLUDED"; private static final SimpleDateFormat sDateFormat = new SimpleDateFormat("yyyy-MM-dd"); + private Lock deactivateSessionsLock = new ReentrantLock(); + private Object wsCountLock = new Object(); + /** * Number of minutes since last license refresh to wait before doing it again */ @@ -252,7 +262,7 @@ * @see ActivationKey#getInstance(boolean) * */ - public static synchronized ActivationKey getInstance() { + public static ActivationKey getInstance() { return getInstance(false); } @@ -266,7 +276,7 @@ * refresh license if needed to * */ - public static synchronized ActivationKey getInstance(boolean refreshIfNeeded) { + public static ActivationKey getInstance(boolean refreshIfNeeded) { if (refreshIfNeeded) { instance.refreshIfNeeded(); } @@ -313,7 +323,7 @@ } } - private void loadFromDB() { + private synchronized void loadFromDB() { org.openbravo.model.ad.system.System sys = OBDal.getInstance().get( org.openbravo.model.ad.system.System.class, "0"); strPublicKey = sys.getInstanceKey(); @@ -328,7 +338,7 @@ loadRestrictions(); } - private void loadInfo(String activationKey) { + private synchronized void loadInfo(String activationKey) { reset(); if (strPublicKey == null || activationKey == null || strPublicKey.equals("") @@ -1072,41 +1082,69 @@ * PING_TIMEOUT_SECS is hardcoded to 120s, pings are hardcoded in front-end to 50s. */ private boolean deactivateTimeOutSessions(String currentSessionId) { - // Last valid ping time is current time substract timeout seconds - Calendar cal = Calendar.getInstance(); - cal.add(Calendar.SECOND, (-1) * PING_TIMEOUT_SECS); - Date lastValidPingTime = new Date(cal.getTimeInMillis()); + if (!deactivateSessionsLock.tryLock()) { + // another thread is already trying to deactivate sessions, don't do anything + return false; + } + try { + // Last valid ping time is current time substract timeout seconds + Calendar cal = Calendar.getInstance(); + cal.add(Calendar.SECOND, (-1) * PING_TIMEOUT_SECS); + Date lastValidPingTime = new Date(cal.getTimeInMillis()); - OBCriteria<Session> obCriteria = OBDal.getInstance().createCriteria(Session.class); + OBCriteria<Session> obCriteria = OBDal.getInstance().createCriteria(Session.class); - // sesion_active='Y' and (lastPing is null or lastPing<lastValidPing) - obCriteria.add(Restrictions.and( - Restrictions.eq(Session.PROPERTY_SESSIONACTIVE, true), - Restrictions.or(Restrictions.isNull(Session.PROPERTY_LASTPING), - Restrictions.lt(Session.PROPERTY_LASTPING, lastValidPingTime)))); - obCriteria.add(Restrictions.not(Restrictions.in(Session.PROPERTY_LOGINSTATUS, - NO_CU_SESSION_TYPES))); + // sesion_active='Y' and (lastPing is null or lastPing<lastValidPing) + obCriteria.add(Restrictions.and( + Restrictions.eq(Session.PROPERTY_SESSIONACTIVE, true), + Restrictions.or(Restrictions.isNull(Session.PROPERTY_LASTPING), + Restrictions.lt(Session.PROPERTY_LASTPING, lastValidPingTime)))); + obCriteria.add(Restrictions.not(Restrictions.in(Session.PROPERTY_LOGINSTATUS, + NO_CU_SESSION_TYPES))); - if (currentSessionId != null) { - obCriteria.add(Restrictions.ne(Session.PROPERTY_ID, currentSessionId)); + if (currentSessionId != null) { + obCriteria.add(Restrictions.ne(Session.PROPERTY_ID, currentSessionId)); + } + + List<Session> exipiredCandidates = obCriteria.list(); + ArrayList<String> sessionsToDeactivate = new ArrayList<>(exipiredCandidates.size()); + for (Session expiredSession : exipiredCandidates) { + if (shouldDeactivateSession(expiredSession, lastValidPingTime)) { + sessionsToDeactivate.add(expiredSession.getId()); + log4j.info("Deactivating session: " + expiredSession.getId() + + " beacuse of ping time out. Last ping: " + expiredSession.getLastPing() + + ". Last valid ping time: " + lastValidPingTime); + } + } + + if (!sessionsToDeactivate.isEmpty()) { + // deactivate sessions is a separate DB trx not to hold locks for a long time + DalConnectionProvider cp = new DalConnectionProvider(false); + Connection trxConn = null; + boolean success = false; + try { + trxConn = cp.getTransactionConnection(); + ActivationKeyData.deactivateSessions(trxConn, cp, + Utility.arrayListToString(sessionsToDeactivate, true)); + cp.releaseCommitConnection(trxConn); + success = true; + } catch (NoConnectionAvailableException | SQLException | ServletException e) { + log.error("couldn't deactivate timed out sessions: " + sessionsToDeactivate, e); + } finally { + if (!success && trxConn != null) { + try { + cp.releaseRollbackConnection(trxConn); + } catch (SQLException e) { + log.error("couldn't rollback failed trx to deactivate timed out sessions: " + + sessionsToDeactivate, e); + } + } + } + } + return !sessionsToDeactivate.isEmpty(); + } finally { + deactivateSessionsLock.unlock(); } - - boolean sessionDeactivated = false; - for (Session expiredSession : obCriteria.list()) { - if (shouldDeactivateSession(expiredSession, lastValidPingTime)) { - expiredSession.setSessionActive(false); - sessionDeactivated = true; - log4j.info("Deactivated session: " + expiredSession.getId() - + " beacuse of ping time out. Last ping: " + expiredSession.getLastPing() - + ". Last valid ping time: " + lastValidPingTime); - } - } - if (sessionDeactivated) { - OBDal.getInstance().flush(); - } else { - log4j.debug("No ping timeout sessions"); - } - return sessionDeactivated; } /** @@ -1118,7 +1156,9 @@ HttpSession session = SessionListener.getActiveSession(sessionId); if (session == null) { log4j.debug("Session " + sessionId + " not found in context"); - return true; + // we cannot deactivate this session because it might have been created in a different node + // from cluster and we cannot know when was used last time + return false; } Date lastRequestTime = new Date(session.getLastAccessedTime()); log4j.debug("Last request received from session " + sessionId + ": " + lastRequestTime); @@ -1830,7 +1870,7 @@ * @param updateCounter * daily calls should be updated */ - public synchronized WSRestriction checkNewWSCall(boolean updateCounter) { + public WSRestriction checkNewWSCall(boolean updateCounter) { if (hasExpired) { return WSRestriction.EXPIRED; } @@ -1850,55 +1890,61 @@ } long checkCalls = maxWsCalls; + long currentDayCount; if (updateCounter) { - wsDayCounter += 1; + currentDayCount = wsDayCounter.incrementAndGet(); // Adding 1 to maxWsCalls because session is already saved in DB checkCalls += 1; + } else { + currentDayCount = wsDayCounter.get(); } - if (wsDayCounter > checkCalls) { - // clean up old days - while (!exceededInLastDays.isEmpty() - && exceededInLastDays.get(0).getTime() < today.getTime() - WS_MS_EXCEEDING_ALLOWED_PERIOD) { - Date removed = exceededInLastDays.remove(0); - log.info("Removed date from exceeded days " + removed); - } + if (currentDayCount > checkCalls) { + synchronized (wsCountLock) { + // clean up old days + while (!exceededInLastDays.isEmpty() + && exceededInLastDays.get(0).getTime() < today.getTime() + - WS_MS_EXCEEDING_ALLOWED_PERIOD) { + Date removed = exceededInLastDays.remove(0); + log.info("Removed date from exceeded days " + removed); + } - if (!exceededInLastDays.contains(today)) { - exceededInLastDays.add(today); + if (!exceededInLastDays.contains(today)) { + exceededInLastDays.add(today); - // Adding a new failing day, send a new beat to butler - Runnable sendBeatProcess = new Runnable() { - @Override - public void run() { - try { - String content = "beatType=CWSR"; - content += "&systemIdentifier=" - + URLEncoder.encode(SystemInfo.getSystemIdentifier(), "utf-8"); - content += "&dbIdentifier=" - + URLEncoder.encode(SystemInfo.getDBIdentifier(), "utf-8"); - content += "&macId=" + URLEncoder.encode(SystemInfo.getMacAddress(), "utf-8"); - content += "&obpsId=" + URLEncoder.encode(SystemInfo.getOBPSInstance(), "utf-8"); - content += "&instanceNo=" - + URLEncoder.encode(SystemInfo.getOBPSIntanceNumber(), "utf-8"); + // Adding a new failing day, send a new beat to butler + Runnable sendBeatProcess = new Runnable() { + @Override + public void run() { + try { ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
