murblanc commented on a change in pull request #1504: URL: https://github.com/apache/lucene-solr/pull/1504#discussion_r428457066
########## File path: solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java ########## @@ -382,45 +383,78 @@ static MapWriter loggingInfo(Policy policy, SolrCloudManager cloudManager, Sugge } public enum Status { - NULL, - //it is just created and not yet used or all operations on it has been completed fully - UNUSED, - COMPUTING, EXECUTING + /** + * A command is actively using and modifying the session to compute placements + */ + COMPUTING, + /** + * A command is not done yet processing its changes but no longer updates or even uses the session + */ + EXECUTING } /** - * This class stores a session for sharing purpose. If a process creates a session to - * compute operations, - * 1) see if there is a session that is available in the cache, - * 2) if yes, check if it is expired - * 3) if it is expired, create a new session - * 4) if it is not expired, borrow it - * 5) after computing operations put it back in the cache + * This class stores sessions for sharing purposes. If a process requires a session to + * compute operations: + * <ol> + * <li>see if there is an available non expired session in the cache,</li> + * <li>if yes, borrow it.</li> + * <li>if no, create a new one and borrow it.</li> + * <li>after computing (update) operations are done, {@link #returnSession(SessionWrapper)} back to the cache so it's + * again available for borrowing.</li> + * <li>after all borrowers are done computing then executing with the session, {@link #release(SessionWrapper)} it, + * which removes it from the cache.</li> + * </ol> */ static class SessionRef { + /** + * Lock protecting access to {@link #sessionWrapperSet} and to {@link #creationsInProgress} + */ private final Object lockObj = new Object(); - private SessionWrapper sessionWrapper = SessionWrapper.DEFAULT_INSTANCE; + /** + * Sessions currently in use in {@link Status#COMPUTING} or {@link Status#EXECUTING} states. As soon as all + * uses of a session are over, that session is removed from this set. Sessions not actively in use are NOT kept around. + * + * <p>Access should only be done under the protection of {@link #lockObj}</p> + */ + private Set<SessionWrapper> sessionWrapperSet = Collections.newSetFromMap(new IdentityHashMap<>()); + + + /** + * Number of sessions currently being created but not yeet present in {@link #sessionWrapperSet}. + * + * <p>Access should only be done under the protection of {@link #lockObj}</p> + */ + private int creationsInProgress = 0; public SessionRef() { } - - //only for debugging - SessionWrapper getSessionWrapper() { - return sessionWrapper; + // used only by tests + boolean isEmpty() { + synchronized (lockObj) { + return sessionWrapperSet.isEmpty(); + } } /** * All operations suggested by the current session object * is complete. Do not even cache anything */ private void release(SessionWrapper sessionWrapper) { + boolean present; synchronized (lockObj) { - if (sessionWrapper.createTime == this.sessionWrapper.createTime && this.sessionWrapper.refCount.get() <= 0) { - log.debug("session set to NULL"); - this.sessionWrapper = SessionWrapper.DEFAULT_INSTANCE; - } // else somebody created a new session b/c of expiry . So no need to do anything about it + present = sessionWrapperSet.remove(sessionWrapper); + } + if (!present) { + log.warn("released session {} not found in session set", sessionWrapper.getCreateTime()); + } else { + TimeSource timeSource = sessionWrapper.session.cloudManager.getTimeSource(); Review comment: Look ok to me ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org