aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1700718785
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -104,46 +103,52 @@ public CollectionPropertiesZkStateReader(ZkStateReader
zkStateReader) {
* @return a map representing the key/value properties for the collection.
*/
public Map<String, String> getCollectionProperties(final String collection,
long cacheForMillis) {
- synchronized (watchedCollectionProps) { // synchronized on the specific
collection
- Watcher watcher = null;
- if (cacheForMillis > 0) {
- watcher =
- collectionPropsWatchers.compute(
- collection,
- (c, w) ->
- w == null ? new PropsWatcher(c, cacheForMillis) :
w.renew(cacheForMillis));
- }
- VersionedCollectionProps vprops = watchedCollectionProps.get(collection);
- boolean haveUnexpiredProps = vprops != null && vprops.cacheUntilNs >
System.nanoTime();
- long untilNs =
- System.nanoTime() + TimeUnit.NANOSECONDS.convert(cacheForMillis,
TimeUnit.MILLISECONDS);
- Map<String, String> properties;
+ Watcher watcher = null; // synchronized on the specific collection
Review Comment:
Reduce the scope of synchronization until it is actually required, i.e when
cached properties expire and need to be fetched from Zookeeper
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -104,46 +103,52 @@ public CollectionPropertiesZkStateReader(ZkStateReader
zkStateReader) {
* @return a map representing the key/value properties for the collection.
*/
public Map<String, String> getCollectionProperties(final String collection,
long cacheForMillis) {
- synchronized (watchedCollectionProps) { // synchronized on the specific
collection
- Watcher watcher = null;
- if (cacheForMillis > 0) {
- watcher =
- collectionPropsWatchers.compute(
- collection,
- (c, w) ->
- w == null ? new PropsWatcher(c, cacheForMillis) :
w.renew(cacheForMillis));
- }
- VersionedCollectionProps vprops = watchedCollectionProps.get(collection);
- boolean haveUnexpiredProps = vprops != null && vprops.cacheUntilNs >
System.nanoTime();
- long untilNs =
- System.nanoTime() + TimeUnit.NANOSECONDS.convert(cacheForMillis,
TimeUnit.MILLISECONDS);
- Map<String, String> properties;
+ Watcher watcher = null; // synchronized on the specific collection
+ if (cacheForMillis > 0) {
+ watcher =
+ collectionPropsWatchers.compute(
+ collection,
+ (c, w) -> w == null ? new PropsWatcher(c, cacheForMillis) :
w.renew(cacheForMillis));
+ }
+ VersionedCollectionProps vprops = watchedCollectionProps.get(collection);
+ boolean haveUnexpiredProps = vprops != null && vprops.cacheUntilNs >
System.nanoTime();
+ long untilNs =
+ System.nanoTime() + TimeUnit.NANOSECONDS.convert(cacheForMillis,
TimeUnit.MILLISECONDS);
+ if (haveUnexpiredProps) {
+ vprops.cacheUntilNs = Math.max(vprops.cacheUntilNs, untilNs);
+ return vprops.props;
+ }
+ // Synchronize only when properties are expired or not present
+ Object collectionLock = getCollectionLock(collection);
+ synchronized (collectionLock) {
Review Comment:
Reduce the scope of synchronization until it is actually required, i.e when
cached properties expire and need to be fetched from Zookeeper
--
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.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
