Skip to content

SOLR-18557: Restore source collection when REINDEXCOLLECTION cleanup fails - #5186

Merged
epugh merged 1 commit into
apache:mainfrom
SpookyJumpyBeans:reindex-abort-readonly
Oct 20, 2026
Merged

epugh merged 1 commit into
apache:mainfrom
SpookyJumpyBeans:reindex-abort-readonly

Conversation

@SpookyJumpyBeans

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/SOLR-18557

Description

When a REINDEXCOLLECTION is aborted or fails, cleanup() deletes the target and checkpoint collections before it turns readOnly off on the source and removes the reindexing state. If deleting the target throws (for example, because an alias references it), the source collection stays read-only.

Solution

The readOnly reset and removeReindexingState now run in a finally block, so the source collection is restored even if the temporary collections can't be deleted. The delete error is still rethrown, so the async request is still reported as failed with the alias message.

try {
  // delete target collection
  // delete checkpoint collection
} finally {
  // turn readOnly off on the source collection
  removeReindexingState(collection);
}

Most of the diff is re-indentation; viewing it with "Hide whitespace" shows the actual change.

Notes:

  • If deleting the target fails, the checkpoint collection is not deleted either. That one is self-healing, because the next REINDEXCOLLECTION run deletes an existing checkpoint collection before starting.
  • The aliased target collection is intentionally left in place, since something else references it.
  • In the in-place mode, the command's own <source> -> <target> alias can also block the delete if a failure happens after it is created. This PR restores the source in that case too, but it doesn't remove that alias; whether cleanup should undo it seems like a separate question.
  • If the readOnly reset itself also fails, its exception replaces the delete error (plain finally semantics). I kept it simple since the reset is the more important failure to surface.

Tests

Added ReindexCollectionTest.testAbortWithAliasedTarget:

  1. Pause a reindex at the existing latch.
  2. Alias the target.
  3. Abort the reindex.
  4. Check that the source is writable again, that its reindexing state is cleared, and that the async request reports FAILED.

The test fails on main ("source collection is in wrong state", with "is part of aliases" in the log) and passes with the fix. All 7 tests in ReindexCollectionTest pass, and so does ./gradlew :solr:core:check -x test.

Checklist

  • I have reviewed the guidelines for How to Contribute 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 to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • 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
  • I have added a changelog entry for my change

…fails

If deleting the target or checkpoint collection threw during cleanup
(for example because an alias references the target), the readOnly
reset and the reindexing state removal were skipped, leaving the source
collection read-only. Run them in a finally block so the source is
always restored, and keep rethrowing the delete error.
@epugh

epugh commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

after starting two nodes bin/solr start -e cloud, this script gave on main the error, and passed with this patch

reindex-abort-repro.sh

@epugh epugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after manual testing.

@epugh epugh self-assigned this Oct 20, 2026
@SpookyJumpyBeans

Copy link
Copy Markdown
Contributor Author

Thanks for testing it manually with a real cluster, @epugh, much appreciated!

@epugh epugh added this to the 10.x milestone Oct 20, 2026
@epugh
epugh merged commit f9a2654 into apache:main Oct 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants