You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "linxiaokun528 (via GitHub)" <gi...@apache.org> on 2026/07/01 02:38:17 UTC
[PR] SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
linxiaokun528 opened a new pull request, #4577:
URL: https://github.com/apache/solr/pull/4577
<!--
_(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
Curator's RECONNECTED event is different from the previous RECONNECTED event. Before Solr10, the OnReconnect is only triggered after a reconnection from a session expiration. Check the following
https://github.com/apache/solr/blob/fdb5314279657f7895a90123436d834e81ea3157/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java#L165 https://github.com/apache/solr/blob/fdb5314279657f7895a90123436d834e81ea3157/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java#L199
But Curator's RECONNECTED event is triggered every time a Solr node is disconnected from a ZooKeeper instance and reconnected to another ZooKeeper instance.
Therefore, currently ZkController.onReconnect is invoked every time a Solr node reconnects to a ZooKeeper instance without a session expiration, which is a huge overhead, especially when we need to rolling-restart a ZooKeeper Cluster. It can take more than 10 minutes for a small Solr cluster to level out.
Similiarly, now ZkController.onDisconnect is triggered just after a disconnection from a Zookeeper instance. It should only be triggered after a session expiration.
# Solution
I created a SolrCuratorEvent class as a Bridge. Now it only has two event types: EXPIRED_RECONNECTION and SESSION_EXPIRATION.
EXPIRED_RECONNECTION: Will only be triggered when Solr reconnects Zookeeper from a session expiration
SESSION_EXPIRATION: Will be triggered when a session expiration happens (The same to Curator's LOST event.)
We should use SolrCuratorEvent instead of Curator's event and we can add more types into it whenever needed.
I also renamed some methods to make them clearer:
ZkController.onReconnect -> ZkController.onExpiredReconnection
ZkController.onDisconnect -> ZkController.onSessionExpiration
# Tests
In ZkControllerTest.java,
1. I added a method `testZkDisconnectionEvents` to make sure when there is a session expiration, ZkController.onSessionExpiration is triggered.
2. I added a method `testZkReconnectionEvents` to make sure when Solr reconnects to ZooKeeper from a session expiration, ZkController.onExpiredReconnection is triggered.
# 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.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] 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, not available for branches on forks living under an organisation)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] 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)
- [ ] I have added a [changelog entry](https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc) for my change
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3562920104
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.respond();
+ }
+ }
+ }
+ };
+ }
+ },
+
+ SESSION_EXPIRATION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return (client, newState) -> {
+ if (newState == ConnectionState.LOST) {
+ action.respond();
+ }
+ };
+ }
+ };
+
+ public abstract ConnectionStateListener of(EventAction action);
+
+ public interface EventAction {
+ void respond();
Review Comment:
I used `run` at first. But it looks like the `run` of `Runnable`, so I changed it into `respond` to avoid confusion.
Will rename it to `trigger`
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3559245887
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.respond();
+ }
+ }
+ }
+ };
+ }
+ },
+
+ SESSION_EXPIRATION {
Review Comment:
Yea though there is a subtle difference that when you get EXPIRED event you are guaranteed that eventually this session will expire since that is the server telling you the session is expired. I believe Curator's LOST can be determined on the client by curator itself based on a timer in which case you don't have the guarantee that the server will decide to expire the session at all.
An example of how to handle this is to defensively pause activities as a leader when you get a disconnect and then completely abdicate leadership on LOST. Because you've paused leadership activities during this period, it is safer for another node to claim leadership in this time as the disconnection-to-session-loss gap becomes the coordinated handover period. That being said Solr doesn't do this at all and relies on other mechanisms to prevent split-brain, etc.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4963069121
@HoustonPutman you mind giving this a look please?
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 closed pull request #4577: SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily
URL: https://github.com/apache/solr/pull/4577
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3561221897
##########
solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java:
##########
@@ -766,6 +770,123 @@ public void testOverseerEnabledClusterPropertyTrue() throws Exception {
}
}
+ @Test
+ public void testZkReconnectionEvents() throws Exception {
Review Comment:
The `sleep` calls here are quite wasteful. Consider `Timeout::waitFor` and/or `RetryUtil`
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-5293811118
I want to merge this soon. @linxiaokun528 Let me know if you need help resolving the conflict that has appeared since raising this PR.
Thanks @risdenk for taking a look.
cc @HoustonPutman (one more shout out in case you wanted to take a look)
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3844580912
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZookeeperEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrZookeeperEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.trigger();
+ }
+ }
Review Comment:
Curator does have useful abstractions, my experience with the `ModeledFramework` has been positive on Solr-adjacent infrastructure we have (also using curator's clever async package).
However, there is nothing in curator that specifically captures reconnected _after_ session expiry (or `LOST`). You have to write it yourself. The fact that it doesn't exist by this point of zk/curator maturity kind of hints at the fact that we may be using zk in an unconventional way as we've been agreeing about, i.e. Solr relies on this specific state transition to trigger leader re-election and various clean-up routines. Thus, I don't know how easy it will be to unwind that from Solr. I worry that it may not be worth it unless we get some tangible benefit because the risks of getting it wrong are big. That being said, if someone wants to analyze what it would take to change our Zk Controller and Election context logic to be "reconnect-type-agnostic" I'd at least take a look 😄
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-5829631082
Sorry I was busy these weeks and didn't login github.
Since we have a new solution, I will close this PR.
Thank everyone for reviewing.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4910554964
@dsmiley Hi dsmiley, could you retrigger the failed test cases? It seems like a flaky failure.
Actually, I think I know the root cause, and I plan to fix it in another test case.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3562922510
##########
solr/core/src/java/org/apache/solr/schema/ZkIndexSchemaReader.java:
##########
@@ -66,10 +66,10 @@ public void preClose(SolrCore core) {
if (cc.isZooKeeperAware()) {
if (log.isDebugEnabled()) {
log.debug(
- "Removing ZkIndexSchemaReader OnReconnect listener as core {} is shutting down.",
+ "Removing ZkIndexSchemaReader OnExpirationReconnection listener as core {} is shutting down.",
Review Comment:
Right. It's a typo, will fix it.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-5363617153
Ultimately I agree that we are fighting curator here. We should be trying to lean into curator and change the way we handle things, leaving more things to curator recipes, than trying to just do zk stuff through curator. I agree in the meantime we need to fix the bug, but let's do it as easily and small-impact-ly as possible so that we can focus on moving stuff over to the curator land.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3808773625
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZookeeperEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrZookeeperEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.trigger();
+ }
+ }
Review Comment:
> until we can move things to persistent watches.
Would you mind elaborating on this?
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4849789698
Will add a changelog entry later
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3562921060
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
Review Comment:
Good suggesion. Will rename it to SolrZookeerEvent.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3808225042
##########
solr/core/src/java/org/apache/solr/schema/ZkIndexSchemaReader.java:
##########
@@ -225,7 +225,7 @@ void updateSchema(Watcher watcher, int expectedZkVersion)
* the current schema from ZooKeeper.
*/
@Override
- public void onReconnect() {
+ public void trigger() {
Review Comment:
Eventually a lot of these things will go away with things like Persistent Watchers
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZookeeperEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrZookeeperEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.trigger();
+ }
+ }
Review Comment:
So I understand and agree with this logic. However, why can't this logic go into the ZkController instead of the individual ConnectionStateListener? We call the users onReconnect ourselves, the only time we give it to Curator directly is in ZkStateReader. But we can make the fix there as well to keep the isExpired state until we can move things to persistent watches.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3826316589
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZookeeperEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrZookeeperEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.trigger();
+ }
+ }
Review Comment:
Or something even crazier like this: https://curator.apache.org/docs/modeled
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3826305802
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZookeeperEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrZookeeperEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.trigger();
+ }
+ }
Review Comment:
So the ZKStateReader needs to watch for a reconnect just to re-establish watches. If we used [Persistent Watchers](https://curator.apache.org/docs/recipes-persistent-watcher/) for the ZKStateReader, it wouldn't need to listen for reconnects.
This is probably true for a lot of the other usages as well, but this is the most glaring one. (And the only one that doesn't go through ZKController)
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4938295304
Regarding this thread @linxiaokun528 @ercsonusharma
>FWIW, The public OnReconnect/OnDisconnect interfaces published in solrj-zookeeper, so removing them is a breaking change. The bug is purely in the detection logic which is fixable while keeping the interfaces intact.
...
> While OnReconnect is triggered, we use AtomicBoolean.compareAndSet to set it back to false. But in this way, the two methods are tightly coupled.
If you look at when `OnReconnect` is [triggered in 9.X](https://github.com/apache/solr/blob/releases/solr/9.10.1/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java#L156-L200) it is effectively an `OnExpired` hook. I think it always has been until the curator migration so your implementation of on "expired" is compatible with the original `OnReconnect`. The downside is that to not break the API you have to continue this misleading naming convention. One could easily argue this misleading naming convention is what caused this bug in the first place (confusion around what is a reconnection vs what is a session expiry + reconnection). So to echo @dsmiley's point I think from a purely _technical_ standpoint it is better to get rid of `OnReconnect`. Now whether to replace it with a `OnSessionExpiryReconnect` abstract class or use this new enum with anonymous class is another matter. I personally don't have strong feelings either way.
Regarding `OnDisconnect` specifically this is a brand new addition in 10.X so IMO it is ok to drop since it has never really worked (I don't consider 10.0 to really be a working/stable release yet given problems like the one addressed by this 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] SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4941608262
> abstract class
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3561245135
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.respond();
+ }
+ }
+ }
+ };
+ }
+ },
+
+ SESSION_EXPIRATION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return (client, newState) -> {
+ if (newState == ConnectionState.LOST) {
+ action.respond();
+ }
+ };
+ }
+ };
+
+ public abstract ConnectionStateListener of(EventAction action);
+
+ public interface EventAction {
+ void respond();
Review Comment:
`respond` is arguably a strange function for an `Action`. I'd expect something like `run`.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4925166979
Thank you for submitting this. I began taking a look as it is an aggravating factor in another bug I've mentioned this in. I don't love that we are trying to reverse engineer grabular zk transition states from coarser curator events though I personally don't have any better ideas yet. Something about this feels like we are fighting the framework. Though I definitely agree that doing this leader churn on every disconnect is totally unsutainable. It may very well be that solr is ill-equipped to take advantage of curator in its current form.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4941677391
> Regarding this thread @linxiaokun528 @ercsonusharma
>
> > FWIW, The public OnReconnect/OnDisconnect interfaces published in solrj-zookeeper, so removing them is a breaking change. The bug is purely in the detection logic which is fixable while keeping the interfaces intact.
>
> ...
>
> > While OnReconnect is triggered, we use AtomicBoolean.compareAndSet to set it back to false. But in this way, the two methods are tightly coupled.
>
> If you look at when `OnReconnect` is [triggered in 9.X](https://github.com/apache/solr/blob/releases/solr/9.10.1/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java#L156-L200) it is effectively an `OnExpired` hook. I think it always has been until the curator migration so your implementation of on "expired" is compatible with the original `OnReconnect`. The downside is that to not break the API you have to continue this misleading naming convention. One could easily argue this misleading naming convention is what caused this bug in the first place (confusion around what is a reconnection vs what is a session expiry + reconnection). So to echo @dsmiley's point I think from a purely _technical_ standpoint it is better to get rid of `OnReconnect`. Now whether to replace it with a `OnSessionExpiryReconnect` abstract class or use this new enum with anonymous class is another matter. I personally don't have strong feelings either way.
>
> Regarding `OnDisconnect` specifically this is a brand new addition in 10.X so IMO it is ok to drop since it has never really worked (I don't consider 10.0 to really be a working/stable release yet given problems like the one addressed by this PR).
@kotman12 We can't use an abstract class. There are classes already `extends` something and need to implement the event. Previously, `OnReconnect` is an interface, which is fine. But we can't `extends` two classes. But the way, I don't like the previous interface `OnReconnect`, which used a default implementation as the following. It's kind of hacky.
```
default void stateChanged(CuratorFramework client, ConnectionState newState) {
if (ConnectionState.RECONNECTED.equals(newState)) {
onReconnect();
}
}
```
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3562923375
##########
solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java:
##########
@@ -766,6 +770,123 @@ public void testOverseerEnabledClusterPropertyTrue() throws Exception {
}
}
+ @Test
+ public void testZkReconnectionEvents() throws Exception {
Review Comment:
Will do it.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4943199473
@kotman12 Thanks for your suggestions. I modified the code according to your comments. Please check.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3777620551
##########
solr/core/src/java/org/apache/solr/cloud/ShardLeaderElectionContext.java:
##########
@@ -164,7 +164,8 @@ void runLeaderProcess(boolean weAreReplacement) throws KeeperException, Interrup
if (shouldAbort()) {
// Solr is shutting down or the ZooKeeper session expired while waiting for replicas. If the
- // later, we cannot be sure we are still the leader, so we should bail out. The OnReconnect
+ // later, we cannot be sure we are still the leader, so we should bail out. The
+ // OnExpiredReconnection
// handler will re-register the cores and handle a new leadership election.
Review Comment:
nit - if you put line 168 and 169 together and `./gradlew tidy` it will reflow the comment nicely again instead of the extra line break 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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3844702980
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZookeeperEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrZookeeperEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.trigger();
+ }
+ }
Review Comment:
My main point is that although a persistent watcher automatically resets a watch after session expiry and reconnect, there are other, non-Watcher related things that also rely on this transition in Solr.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3555935729
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.respond();
+ }
+ }
+ }
+ };
+ }
+ },
+
+ SESSION_EXPIRATION {
Review Comment:
Great information! Thanks a lot. And you make me realize that even with the native zookeeper client, after an `Watcher.Event.KeeperState.Expired` event is received, the ephemeral nodes may still exists. Technically, the workflow should be like the following
[Session Expiration Triggered]
│
â–¼
1. The Leader node initiates a transaction proposal to delete the ephemeral nodes.
│
â–¼
2. The proposal is broadcasted across the cluster and confirmed by a quorum of nodes.
│
â–¼
3. Each Follower node commits the transaction locally, deleting the ephemeral nodes from memory.
When Solr receives the Expired event, the ephemeral nodes might not have been deleted yet due to propagation delay within the ZooKeeper cluster. Therefore, we need to do some defensive programming.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3561235422
##########
solr/core/src/java/org/apache/solr/schema/ZkIndexSchemaReader.java:
##########
@@ -66,10 +66,10 @@ public void preClose(SolrCore core) {
if (cc.isZooKeeperAware()) {
if (log.isDebugEnabled()) {
log.debug(
- "Removing ZkIndexSchemaReader OnReconnect listener as core {} is shutting down.",
+ "Removing ZkIndexSchemaReader OnExpirationReconnection listener as core {} is shutting down.",
Review Comment:
I feel we should pick one `OnExpiration...` vs `OnExpired...` and be consistent. It doesn't seem that `OnExpirationReconnection` is any kind of official entity anyway.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3562920104
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.respond();
+ }
+ }
+ }
+ };
+ }
+ },
+
+ SESSION_EXPIRATION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return (client, newState) -> {
+ if (newState == ConnectionState.LOST) {
+ action.respond();
+ }
+ };
+ }
+ };
+
+ public abstract ConnectionStateListener of(EventAction action);
+
+ public interface EventAction {
+ void respond();
Review Comment:
I used `run` at first. But it looks like the `run` of `Runnable`, so I changed it into `respond` to avoid confusion.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4873248778
Just a minor response to Sonu's comment:
> FWIW, The public OnReconnect/OnDisconnect interfaces published in solrj-zookeeper, so removing them is a breaking change.
solrj-zookeeper is largely SolrCloud internals that only needs to be a client consumable JAR for those users who use it indirectly via a Zk based ClusterStateProvider. Maybe there are some gray areas. So if we think a (technically) breaking change here is better -- go for it.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4882338206
Sorry, I had to force push because I used the wrong author/email. But the code is exactly the same.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4931301267
> Thank you for submitting this. I began taking a look as it is an aggravating factor in another bug I've mentioned this in. I don't love that we are trying to reverse engineer granular zk transition states from coarser curator events though I personally don't have any better ideas yet. Something about this feels like we are fighting the framework. Though I definitely agree that doing this leader churn on every disconnect is totally unsutainable. It may very well be that solr is ill-equipped to take advantage of curator in its current form.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 closed pull request #4577: SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily
URL: https://github.com/apache/solr/pull/4577
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3561262156
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
Review Comment:
I would call this a `SolrZookeerEvent` or `ZookeeperEvent`. Conceptually these are not Curator-specific events. Even if the curator implementation is triggering them now they could have been ported from a different ZK-client implementation. In fact you are _nearly exactly_ reproducing the behavior from 9.X which ran without curator.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-5009950300
@kotman12 It's been 2 weeks. Should we merge this 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] SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-5012854622
> @kotman12 It's been 2 weeks. Should we merge this PR?
I don't feel there is any big rush for this since we don't even have a 10.1 release brunch cut. This will get merged before next release. I'd like to get @HoustonPutman or @risdenk to take a look at this since they drove a bulk of the curator migration.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-5013821289
> > @kotman12 It's been 2 weeks. Should we merge this PR?
>
> I don't feel there is any big rush for this since we don't even have a 10.1 release brunch cut. This will get merged before next release. I'd like to get @HoustonPutman or @risdenk to take a look at this since they drove a bulk of the curator migration.
Sure. Thanks very much.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4882292574
> Thanks for tracking this down. It's fine to keep the current direction if you want the clean removal, but flagging that the same fix is achievable with much less surface change IMO.
>
> `SolrCuratorEvent` is more machinery than needed. It's an enum + abstract of() + a new EventAction type that's really just Runnable renamed. The same behavior can live directly on the existing OnReconnect/OnDisconnect callbacks and no new type. The AtomicBoolean that makes it correct just lives in the returned closure.
>
> FWIW, The public OnReconnect/OnDisconnect interfaces published in solrj-zookeeper, so removing them is a breaking change. The bug is purely in the detection logic which is fixable while keeping the interfaces intact.
@ercsonusharma Thanks for reviewing.
I created the new type EventAction/SolrCuratorEvent mainly for future usage. Curator does not contain events like expiredReconnect. Since we need different events from those provided by Curator, I think we should create our own event types.
> The same behavior can live directly on the existing OnReconnect/OnDisconnect callbacks and no new type.
The only way I can figure out this is to have an AtomicBoolean variable. When OnDisconnect is triggered, the variable is set to `true`. While OnReconnect is triggered, we use `AtomicBoolean.compareAndSet` to set it back to `false`. But in this way, the two methods are tightly coupled. Besides, as I mentioned above, we may need an event like expiredReconnect again in the future (or maybe now. We may have other places suffering the same kind of issue.) I think we should have an easier way to deal with this kind of event, instead of using AtomicBoolean every time.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "kotman12 (via GitHub)" <gi...@apache.org>.
kotman12 commented on code in PR #4577:
URL: https://github.com/apache/solr/pull/4577#discussion_r3551607937
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrCuratorEvent.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.cloud;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
+
+public enum SolrCuratorEvent {
+ // Only triggered after resume from expiration
+ EXPIRED_RECONNECTION {
+ @Override
+ public ConnectionStateListener of(EventAction action) {
+ return new ConnectionStateListener() {
+ private final AtomicBoolean isExpired = new AtomicBoolean(false);
+
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState) {
+ if (newState == ConnectionState.LOST) {
+ isExpired.set(true);
+ } else if (newState == ConnectionState.RECONNECTED) {
+ if (isExpired.compareAndSet(true, false)) {
+ action.respond();
+ }
+ }
+ }
+ };
+ }
+ },
+
+ SESSION_EXPIRATION {
Review Comment:
One thing to keep in mind is that `LOST` technically does not guarantee the session to be expired (although it very likely is). Curator triggers this state on a timer. I only mention this because some code may subtly rely on the clean-up of ephemeral nodes when this transition is reached which may not happen. One could look at sessionId to determine this though OTOH that would feel like we're fighting the curator framework. Mainly calling this out for visibility.
https://curator.apache.org/docs/tech-note-12/
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 commented on PR #4577:
URL: https://github.com/apache/solr/pull/4577#issuecomment-4931356806
> Thank you for submitting this. I began taking a look as it is an aggravating factor in another bug I've mentioned this in. I don't love that we are trying to reverse engineer granular zk transition states from coarser curator events though I personally don't have any better ideas yet. Something about this feels like we are fighting the framework. Though I definitely agree that doing this leader churn on every disconnect is totally unsutainable. It may very well be that solr is ill-equipped to take advantage of curator in its current form.
@kotman12 Oops, clicked 'Close' by mistake! Reopening. Here are my thoughts on your previous comments...
Anyway here is my take: I don't necessarily view this as fighting the framework; rather, it’s more of a necessary enhancement/workaround to bridge the framework's operational gaps.
As a side note, I did identify the "Zombie Leader" problem as well. I full plan to tackle that root cause in a subsequent PR after this one. While this current PR doesn’t completely eliminate the issue, it drastically reduces the likelihood of triggering it in our environment.
Given that Solr 10 is currently causing severe stability issues in our environment, could we consider merging this PR as a tactical mitigation for now? Investigating a brand-new approach or fully fixing the Zombie Leader bug will undoubtedly be a long-term effort that could span multiple Solr releases, whereas we desperately need relief right now.
--
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-18298: ZkController.OnReconnect Is Trigger Unnecessarily [solr]
Posted by "linxiaokun528 (via GitHub)" <gi...@apache.org>.
linxiaokun528 closed pull request #4577: SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily
URL: https://github.com/apache/solr/pull/4577
--
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