You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "aparnasuresh85 (via GitHub)" <gi...@apache.org> on 2024/08/01 19:23:11 UTC
[PR] Reduce thread contention in ZkStateReader.getCollectionProperties [solr]
aparnasuresh85 opened a new pull request, #2611:
URL: https://github.com/apache/solr/pull/2611
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
Building on <https://github.com/apache/solr/pull/2585|PR> which delegated Collection Properties Management to `CollectionPropertiesZkStateReader`, this PR seeks to minimize thread contention within `CollectionPropertiesZkStateReader`.
# Solution
Please provide a short description of the approach taken to implement your solution.
- Use collection level locking where relevant instead of synchronizing on "watchedCollectionProperties"
- With the double checked locking implemented in CollectionPropertiesZkStateReader, the scope of the synchronized (this) would be naturally reduced to collection property operations, and no longer be in contention with the synchronization of unrelated operations on ZkStateReader.
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
Ensured existing tests pass
# Checklist
Please review the following and check all that apply:
- [X] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1708536459
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -391,13 +383,7 @@ public void run() {
private class CacheCleaner implements Runnable {
@Override
public void run() {
- while (!Thread.interrupted()) {
- try {
- Thread.sleep(60000);
- } catch (InterruptedException e) {
- // Executor shutdown will send us an interrupt
- break;
- }
+ if (!closed) {
Review Comment:
don't even need this condition. The work is not contingent on being closed or not AFAICT. Even if it did, the pool is going to be closed when it's being shut down so it's okay.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on PR #2611:
URL: https://github.com/apache/solr/pull/2611#issuecomment-2263809638
cc @dsmiley @pmcarthur-apache @pjmcarthur
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1707659461
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
I tried this. Using the following code, where cacheCleanerExecutor is defined as
```
private final ScheduledExecutorService cacheCleanerExecutor =
Executors.newSingleThreadScheduledExecutor(new SolrNamedThreadFactory("cacheCleaner"));
```:
and then used in `fetchCollectionProperties()` as follows:
```
if (cacheCleanerExecutor.isShutdown()) {
synchronized (this) {
if (cacheCleanerExecutor.isShutdown()) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
}
```
I ran a Junit test and set a breakpoint at `cacheCleanerExecutor.scheduleAtFixedRate`, it is never hit.
But when I changed to using a state variable like the following, I see the breakpoint being hit to schedule the cacheCleaner thread
```
if (collectionPropsCacheCleanerInitialized.compareAndSet(false, true)) {
synchronized (this) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1707659461
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
I tried this. Using the following code, where cacheCleanerExecutor is defined as
```
private final ScheduledExecutorService cacheCleanerExecutor =
Executors.newSingleThreadScheduledExecutor(new SolrNamedThreadFactory("cacheCleaner"));
```
and then used in `fetchCollectionProperties()` as follows:
```
if (cacheCleanerExecutor.isShutdown()) {
synchronized (this) {
if (cacheCleanerExecutor.isShutdown()) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
}
```
I ran a Junit test and set a breakpoint at `cacheCleanerExecutor.scheduleAtFixedRate`, it is never hit.
But when I changed to using a state variable like the following, where `collectionPropsCacheCleanerInitialized` is an `AtomicBoolean`, I see the breakpoint being hit to schedule the cacheCleaner thread
```
if (collectionPropsCacheCleanerInitialized.compareAndSet(false, true)) {
synchronized (this) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1706392465
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
If you want.
But I think the best solution reflecting the pattern/need for this cache cleaner is a ScheduledExecutorService and no extra state var. all ExecutorService's *have* state or could use null.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1704608234
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -154,8 +150,14 @@ public Map<String, String> getCollectionProperties(final String collection, long
@Override
public void close() {
this.closed = true;
- notifications.shutdownNow();
- ExecutorUtil.shutdownAndAwaitTermination(notifications);
+ if (cacheCleanerThread != null) {
Review Comment:
synchronized?
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -72,11 +72,7 @@ public class CollectionPropertiesZkStateReader implements Closeable {
ExecutorUtil.newMDCAwareSingleThreadExecutor(
new SolrNamedThreadFactory("collectionPropsNotifications"));
- private final ExecutorService notifications =
- ExecutorUtil.newMDCAwareCachedThreadPool("cachecleaner");
-
- // identify if the cleaner has already been started.
- private final AtomicBoolean collectionPropsCacheCleanerInitialized = new AtomicBoolean(false);
+ private Thread cacheCleanerThread;
Review Comment:
Must be volatile if you wish to use double-checked locking idiom, as you are doing. Alternatively you could always create it and instead use double-check pattern to start if not started. Up to you.
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -154,8 +150,14 @@ public Map<String, String> getCollectionProperties(final String collection, long
@Override
public void close() {
this.closed = true;
- notifications.shutdownNow();
- ExecutorUtil.shutdownAndAwaitTermination(notifications);
+ if (cacheCleanerThread != null) {
+ cacheCleanerThread.interrupt();
+ try {
+ cacheCleanerThread.join();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
Review Comment:
I would say don't set interruption during closing. Add a comment with the rationale like "ignore since we are closing"
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1707659461
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
I tried this. Using the following code, where cacheCleanerExecutor is defined as
```
private final ScheduledExecutorService cacheCleanerExecutor =
Executors.newSingleThreadScheduledExecutor(new SolrNamedThreadFactory("cacheCleaner"));
```
and then used in `fetchCollectionProperties()` as follows:
```
if (cacheCleanerExecutor.isShutdown()) {
synchronized (this) {
if (cacheCleanerExecutor.isShutdown()) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
}
```
I ran a Junit test and set a breakpoint at `cacheCleanerExecutor.scheduleAtFixedRate`, it is never hit.
But when I changed to using a state variable like the following, I see the breakpoint being hit to schedule the cacheCleaner thread
```
if (collectionPropsCacheCleanerInitialized.compareAndSet(false, true)) {
synchronized (this) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17396: Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #2611:
URL: https://github.com/apache/solr/pull/2611
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17396: Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1709653157
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -391,13 +383,7 @@ public void run() {
private class CacheCleaner implements Runnable {
@Override
public void run() {
- while (!Thread.interrupted()) {
- try {
- Thread.sleep(60000);
- } catch (InterruptedException e) {
- // Executor shutdown will send us an interrupt
- break;
- }
+ if (!closed) {
Review Comment:
Updated.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1706005573
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
Can we instead revert to the logic 2 commits prior that used the ExecutorService and the Atomic Boolean to ensure the `cacheCleaner` is initialized only once?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1702180575
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -217,7 +222,8 @@ public void process(WatchedEvent event) {
*/
void refreshAndWatch(boolean notifyWatchers) {
try {
- synchronized (watchedCollectionProps) { // making decisions based on the result of a get...
Review Comment:
Reinstated the comment
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
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
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1704493967
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -77,12 +75,13 @@ public class CollectionPropertiesZkStateReader implements Closeable {
private final ExecutorService notifications =
ExecutorUtil.newMDCAwareCachedThreadPool("cachecleaner");
- // only kept to identify if the cleaner has already been started.
- private Future<?> collectionPropsCacheCleaner;
+ // identify if the cleaner has already been started.
+ private final AtomicBoolean collectionPropsCacheCleanerInitialized = new AtomicBoolean(false);
Review Comment:
Yes, done :). Visible in the latest commit.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1702323807
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -77,12 +75,13 @@ public class CollectionPropertiesZkStateReader implements Closeable {
private final ExecutorService notifications =
ExecutorUtil.newMDCAwareCachedThreadPool("cachecleaner");
- // only kept to identify if the cleaner has already been started.
- private Future<?> collectionPropsCacheCleaner;
+ // identify if the cleaner has already been started.
+ private final AtomicBoolean collectionPropsCacheCleanerInitialized = new AtomicBoolean(false);
Review Comment:
it'd just be an interrupt & join (2 lines); no?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1700915813
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -47,7 +46,6 @@ public class CollectionPropertiesZkStateReader implements Closeable {
private volatile boolean closed = false;
private final SolrZkClient zkClient;
- private final ZkStateReader zkStateReader;
Review Comment:
glad to see this gone!
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -217,7 +222,8 @@ public void process(WatchedEvent event) {
*/
void refreshAndWatch(boolean notifyWatchers) {
try {
- synchronized (watchedCollectionProps) { // making decisions based on the result of a get...
Review Comment:
the comment is still reasonable; could be it's own line once the lock is obtained
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -217,7 +222,8 @@ public void process(WatchedEvent event) {
*/
void refreshAndWatch(boolean notifyWatchers) {
try {
- synchronized (watchedCollectionProps) { // making decisions based on the result of a get...
+ Object collectionLock = getCollectionLock(coll);
+ synchronized (collectionLock) { // Lock on individual collection
Review Comment:
as one line then no need to declare & name "collectionLock". Also the comment doesn't add anything that is already extremely obvious
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -402,10 +412,11 @@ public void removeCollectionPropsWatcher(String collection, CollectionPropsWatch
v.stateWatchers.remove(watcher);
if (v.canBeRemoved()) {
// don't want this to happen in middle of other blocks that might add it back.
- synchronized (watchedCollectionProps) {
+ Object collectionLock = getCollectionLock(collection);
+ synchronized (collectionLock) {
Review Comment:
combine as one line
##########
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:
combine to one line
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -77,12 +75,13 @@ public class CollectionPropertiesZkStateReader implements Closeable {
private final ExecutorService notifications =
ExecutorUtil.newMDCAwareCachedThreadPool("cachecleaner");
- // only kept to identify if the cleaner has already been started.
- private Future<?> collectionPropsCacheCleaner;
+ // identify if the cleaner has already been started.
+ private final AtomicBoolean collectionPropsCacheCleanerInitialized = new AtomicBoolean(false);
Review Comment:
There is only one cache cleaner; can't it simply be a Thread instead of an ExecutorService? Then we can easily see if it's running or not if needed.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1708537956
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
Looks good!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1702174830
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -77,12 +75,13 @@ public class CollectionPropertiesZkStateReader implements Closeable {
private final ExecutorService notifications =
ExecutorUtil.newMDCAwareCachedThreadPool("cachecleaner");
- // only kept to identify if the cleaner has already been started.
- private Future<?> collectionPropsCacheCleaner;
+ // identify if the cleaner has already been started.
+ private final AtomicBoolean collectionPropsCacheCleanerInitialized = new AtomicBoolean(false);
Review Comment:
Yes - I was wondering using an ExecutorService is easier, because of the number of lines of code added to stop the standalone thread when CPZSR closes. Let me know.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1707659461
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
I tried this. Using the following code, where cacheCleanerExecutor is defined as
```
private final ScheduledExecutorService cacheCleanerExecutor =
Executors.newSingleThreadScheduledExecutor(new SolrNamedThreadFactory("cacheCleaner"));
```:
and then used in `fetchCollectionProperties()` as follows:
```
if (cacheCleanerExecutor.isShutdown()) {
synchronized (this) {
if (cacheCleanerExecutor.isShutdown()) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
}
```
I ran a Junit test and set a breakpoint at `cacheCleanerExecutor.scheduleAtFixedRate`, it is never hit.
But when I changed to using a state variable like the following, I see the breakpoint being hit to schedule the cacheCleaner thread
```
if (collectionPropsCacheCleanerInitialized.compareAndSet(false, true)) {
synchronized (this) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1707774326
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
fetchCollectionProperties: need to know if there is a task in the executor or not. If we use the ScheduledThreadPoolExecutor type, then we have methods like `getTaskCount` to know if something is scheduled.
An alternative (maybe simpler?) is to use Timer.
Wether the ScheduledExecutor or Timer, could also use a null instance initially (not final, of course) to differentiate not-started from started which is totally fine too.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1708045290
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
Ok using ScheduledThreadPoolExecutor works. I updated the PR.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1707659461
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
I tried this. Using the following code, where cacheCleanerExecutor is defined as
```
private final ScheduledExecutorService cacheCleanerExecutor =
Executors.newSingleThreadScheduledExecutor(new SolrNamedThreadFactory("cacheCleaner"));
```:
and then used in `fetchCollectionProperties()` as follows:
```
if (cacheCleanerExecutor.isShutdown()) {
synchronized (this) {
if (cacheCleanerExecutor.isShutdown()) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
}
```
I ran a Junit test and set a breakpoint at `cacheCleanerExecutor.scheduleAtFixedRate`, it is never hit.
But when I changed to using a state variable like the following, I see the breakpoint being hit to schedule the cacheCleaner thread
```
if (collectionPropsCacheCleanerInitialized.compareAndSet(false, true)) {
synchronized (this) {
cacheCleanerExecutor.scheduleAtFixedRate(new CacheCleaner(), 0, 1, TimeUnit.MINUTES);
}
}
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2611:
URL: https://github.com/apache/solr/pull/2611#discussion_r1704685321
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/CollectionPropertiesZkStateReader.java:
##########
@@ -300,10 +313,11 @@ private VersionedCollectionProps fetchCollectionProperties(String collection, Wa
throws KeeperException, InterruptedException {
final String znodePath = getCollectionPropsPath(collection);
// lazy init cache cleaner once we know someone is using collection properties.
- if (collectionPropsCacheCleaner == null) {
- synchronized (zkStateReader.getUpdateLock()) { // Double-checked locking
- if (collectionPropsCacheCleaner == null) {
- collectionPropsCacheCleaner = notifications.submit(new CacheCleaner());
+ if (cacheCleanerThread == null) {
+ synchronized (this) {
+ if (cacheCleanerThread == null) {
Review Comment:
Probably should check for `!closed` as well so that we don't start this thread after closing.
Come to think of it, the CacheCleaner ends on interruption and it doesn't looked at "closed" at all. That's rather brittle; it should loop while `!closed` and not "break" if Interrupted -- this is the typical approach.
Come to think of it further... hey, that CacheCleaner looks like a "scheduled" job every minute. Could use a ScheduledExecutor and look more "obvious". That's what I recommend. I know that changes things here :-)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17396: Reduce thread contention in ZkStateReader.getCollectionProperties() [solr]
Posted by "aparnasuresh85 (via GitHub)" <gi...@apache.org>.
aparnasuresh85 commented on PR #2611:
URL: https://github.com/apache/solr/pull/2611#issuecomment-2275992159
> Looks essentially ready... albeit missing a JIRA & CHANGES.txt :-)
Created JIRA: https://issues.apache.org/jira/browse/SOLR-17396 and added CHANGES.txt!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org