murblanc commented on a change in pull request #1504:
URL: https://github.com/apache/lucene-solr/pull/1504#discussion_r428457206
##########
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}.
Review comment:
Think different
----------------------------------------------------------------
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
