SpookyJumpyBeans opened a new pull request, #5223: URL: https://github.com/apache/solr/pull/5223
https://issues.apache.org/jira/browse/SOLR-18584 # Description When a RESTORE to a new collection fails while installing the shard data, `RestoreOnANewCollection` leaves the collection behind with all its shards in CONSTRUCTION. A retry then sees an existing collection and uses `RestoreOnExistingCollection`, which restores the data and reports success but never marks the shards ACTIVE, so updates to the collection are rejected. # Solution If anything fails after the collection is created and before its shards are marked ACTIVE, delete the collection and rethrow, the same cleanup that is already done when the initial replicas can't be created. A retry then restores into a new collection again. An async RESTORE doesn't throw when restoring a shard fails, it only records the failure in the results (see the TODO in `ShardRequestTracker.processResponses`). No failure is recorded before that step (the method returns earlier if one is), so a failure recorded after it is treated the same way: ```java failures = results.get("failure"); if (failures != null && ((SimpleOrderedMap<?>) failures).size() > 0) { throw new SolrException(ErrorCode.SERVER_ERROR, "Could not restore shard"); } markAllShardsAsActive(restoreCollection); } catch (Exception e) { // Until its shards are marked active, the collection can't be used. Delete it, ... try { CollectionHandlingUtils.cleanupCollection( rc.adminCmdContext, rc.restoreCollectionName, new NamedList<>(), ccc); } catch (Exception cleanupException) { e.addSuppressed(cleanupException); } throw e; } ``` Notes: - Most of the diff in `RestoreCmd` is re-indentation for the new `try` block; it's much smaller with whitespace ignored. - An `InterruptedException` is rethrown without cleanup (and the interrupt flag restored), since the node is likely shutting down. - I didn't change `RestoreOnExistingCollection` to activate CONSTRUCTION shards: an existing collection can legitimately have shards under construction (e.g. sub-shards during SPLITSHARD). - A failure in `addReplicasToShards`, after the shards are ACTIVE, still leaves the collection in place; it's usable at that point. - Behavior change for async RESTORE to a new collection: when restoring a shard fails, it used to report FAILED but leave an active collection missing that shard's data. It now deletes the collection, like a synchronous RESTORE. The existing-collection path is unchanged. - The restored config set isn't deleted with the collection; a retry finds it and reuses it, as it already does for a config that exists. # Tests Added `LocalFSCloudIncrementalBackupTest.testRetryAfterFailedRestoreToNewCollection`. It backs up a collection and makes the existing `ErrorThrowingTrackingBackupRepository` fail on every node. A synchronous RESTORE to a new collection then fails, and so does an async one; after each, the test checks that the collection was not left behind. It then retries the RESTORE with the repository working and checks that the collection's shards become active and that it has all the documents. Without the fix (and without the cleanup checks), the retry succeeds but waiting for active shards times out with both shards in `construction`. With the fix the test passes. `LocalFSCloudIncrementalBackupTest` passes on 3 runs with different seeds (its `@Nightly` test skipped), `TestLocalFSCloudBackupRestore` passes, and so does `./gradlew :solr:core:check -x test`. # Checklist - [x] I have reviewed the guidelines for How to Contribute 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 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 - [x] I have added a changelog entry 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
