You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "gerlowskija (via GitHub)" <gi...@apache.org> on 2025/03/27 18:12:48 UTC
[PR] SOLR-17692: Abort ongoing fetches on core close [solr]
gerlowskija opened a new pull request, #3292:
URL: https://github.com/apache/solr/pull/3292
https://issues.apache.org/jira/browse/SOLR-17692
# Description
I recently deleted a NRT replica that was in the middle of a full-recovery and was a bit surprised to see that the "delete" blocked waiting for the recovery to finish. This is a minor pain when the index is small, but becomes a huge waste of administrator time (and network bandwidth!) as index sizes grow.
There's some plumbing in Solr that attempts to preempt recovery during a DELETE, but it appears that it mostly comes into play during peer-sync and "background replication" scenarios (i.e. PULL and TLOG replicas that do full-recovery during normal operation). Preemption doesn't seem to work once a recovering core is in the midst of a "full recovery".
`RecoveryStrategy.close()` sets a boolean flag that is checked at various points in the recovery process, and can preempt recovery at those points. But the flag isn't visible to `IndexFetcher`, where the bulk of a full-recovery's time gets spent.
# Solution
This commit aims to fix this by having `RecoveryStrategy.close()` invoke an existing `ReplicationHandler.abortFetch`, which sets a flag that *is* noticed by IndexFetcher. This should ensure that long-running recovery operations don't block DELETEREPLICA or other core-shutdown paths.
# Tests
Manual testing. May need some help figuring out a way to exercise this in an automated test, given the timing-dependent nature of the bug and fix.
# 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.
- [ ] 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] SOLR-17692: Abort ongoing fetches on core close [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #3292:
URL: https://github.com/apache/solr/pull/3292#issuecomment-2759344996
Thanks!
I'd still really like to find a way to get this covered by automated tests, but I can't come up with a good way to cover it. A unit test should be possible but the mocking would be pretty ugly. And idk of a good way to replicate the necessary timing in a higher-level test. (i.e. to ensure that the core-close happens _during_ IndexFetcher's work, and not before or after). Open to suggestions if anyone has them!
--
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-17692: Abort ongoing fetches on core close [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #3292:
URL: https://github.com/apache/solr/pull/3292#discussion_r2017514015
##########
solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java:
##########
@@ -188,6 +189,9 @@ public final void close() {
close = true;
cancelPrepRecoveryCmd();
log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName);
+ if (replicationHandlerDoingFetch != null) {
Review Comment:
[Q] Technically, there's a possibility here that 'replicationHandlerDoingFetch' is unset after we've checked the conditional, but before we call `abortFetch()`. If that happens, it'd lead to a NPE on the `abortFetch()` call. But it seems so remote that I'm not sure it's worth the complexity of guarding against it....
I'll fix it prior to merging, unless others come down on the "not worth the complexity" side of things.
--
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-17692: Abort ongoing fetches on core close [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija merged PR #3292:
URL: https://github.com/apache/solr/pull/3292
--
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-17692: Abort ongoing fetches on core close [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #3292:
URL: https://github.com/apache/solr/pull/3292#discussion_r2017514015
##########
solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java:
##########
@@ -188,6 +189,9 @@ public final void close() {
close = true;
cancelPrepRecoveryCmd();
log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName);
+ if (replicationHandlerDoingFetch != null) {
Review Comment:
[Q] Technically, there's a possibility here that 'replicationHandlerDoingFetch' is unset after we've checked the conditional, but before we call `abortFetch()`. If that happens, it's lead to a NPE on the `abortFetch()` call. But it seems so remote that I'm not sure it's worth the complexity of guarding against it....
##########
solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java:
##########
@@ -188,6 +189,9 @@ public final void close() {
close = true;
cancelPrepRecoveryCmd();
log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName);
+ if (replicationHandlerDoingFetch != null) {
Review Comment:
[Q] Technically, there's a possibility here that 'replicationHandlerDoingFetch' is unset after we've checked the conditional, but before we call `abortFetch()`. If that happens, it'd lead to a NPE on the `abortFetch()` call. But it seems so remote that I'm not sure it's worth the complexity of guarding against 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-17692: Abort ongoing fetches on core close [solr]
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #3292:
URL: https://github.com/apache/solr/pull/3292#discussion_r2017514015
##########
solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java:
##########
@@ -188,6 +189,9 @@ public final void close() {
close = true;
cancelPrepRecoveryCmd();
log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName);
+ if (replicationHandlerDoingFetch != null) {
Review Comment:
[Q] Technically, there's a possibility here that 'replicationHandlerDoingFetch' is unset after we've checked the conditional, but before we call `abortFetch()1. If that happens, it's lead to a NPE on the `abortFetch()` call. But it seems so remote that I'm not sure it's worth the complexity of guarding against 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