SOLR-18391: Clean up a collection when creation fails partway, and name the cause in the error

[nick-boss-tech]

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

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

A create that fails after the collection state has been written now deletes the collection before the error returns (the cases where it cannot are under Limits), and an unexpected failure returns a message that names the cause instead of a null message. Today that cleanup runs for only two kinds of failure, and any other failure leaves behind a collection with no replicas that blocks a retry under the same name. This is the broader route suggested on #4997, which stays as a draft and is superseded by this PR. We would like your view on the three choices below, the main one being whether a failed alias write at the end of a create should delete a collection that is otherwise working.

What happens today

When collection creation fails after the collection state has been written, whether anything is cleaned up depends on the type of the exception. Only two failures run the delete that removes the half-created collection: an AssignmentException out of replica assignment, and a core that fails to come up. Any other failure, for example a NullPointerException when the placement strategy reads a cluster state that does not contain the new collection yet, lands in a catch-all that throws a SolrException with a null message, and the collection stays behind with no replicas. The client cannot tell what failed, and the leftover collection blocks a retry under the same name. The two waits in the create (collection visible, replica entries visible) also leave the collection behind when they time out.

What this change does

• Cleanup. Any exception thrown after the collection state is written now deletes the collection before the error returns (CreateCollectionCmd.java:515-516), so a failed create does not leave the collection behind (the exceptions are under Limits). CreateCollectionCmd records a stateWritten flag (CreateCollectionCmd.java:162) and runs one catch for the whole method (CreateCollectionCmd.java:510). Cleanup failures are logged at ERROR and suppressed (CreateCollectionCmd.java:583-587) so the client always sees the original error.
• Error messages. Unexpected failures return Could not create collection <name>: <cause> (CreateCollectionCmd.java:526) instead of a null message, and the message for a core that fails to come up now names the first per-node failure (CreateCollectionCmd.java:445, :471-477).
• Existing collection. In the non-PRS branch (collections without per-replica state) the command checks ZooKeeper directly for the collection's state.json before the flag is set (CreateCollectionCmd.java:233-236), so a collection that already exists fails fast with BAD_REQUEST collection already exists: <name> and can never be deleted by a create that did not make it.
• Alias write. The alias write at the end of a create (CreateCollectionCmd.java:500-507) goes through a bounded retry: 3 attempts with a 200 ms pause (CreateCollectionCmd.java:103-104). Only ZooKeeperException is retried and interrupts are not retried (CreateCollectionCmd.java:537-565). The cleanup above runs when the retries are exhausted, or at once when the alias write fails with any other exception.
• Stale cluster state. The placement strategy guard from SOLR-18391: Fail collection creation cleanly when the collection is missing from cluster state during replica assignment #4997, an AssignmentException naming the missing collection instead of an NPE (PlacementPluginAssignStrategy.java:179-187), is kept and extended to the system-collection branch (PlacementPluginAssignStrategy.java:204), so the other callers of the strategy get the clearer error too. The non-PRS cluster state re-read now fills in the waited-for collection when the re-read does not contain it yet (CreateCollectionCmd.java:269-271), which closes the stale-read window the ticket hit.

Proof

The new cleanup tests are real-cluster tests with failure injection through a delegating placement plugin factory; the retry helper is covered by unit tests. With the production code at the base (c3cdf7b) and the tests from this branch, leaving out CreateCollectionCmdRetryTest because it calls the new helper and does not compile on base, exactly two tests fail, in the ticket's shapes: testCleanupAfterUnexpectedPlacementFailure (the client gets a SolrException with an empty message) and testAssignForMissingCollection (a NullPointerException instead of an AssignmentException). testCleanupAfterPlacementException passes on base, because main already cleans up after an AssignmentException.

Verified locally 2026-10-05 at fca4235 (the PR head differs from it only by one later commit that removes an internal handoff document): CreateCollectionCleanupTest 4/4 (CreateCollectionCleanupTest.java), PlacementPluginIntegrationTest 8/8 (PlacementPluginIntegrationTest.java), OverseerCollectionConfigSetProcessorTest 26/26 (OverseerCollectionConfigSetProcessorTest.java), DeleteCoreRemnantsOnCreateTest 5/5 (DeleteCoreRemnantsOnCreateTest.java), CreateCollectionCmdRetryTest 4/4 (CreateCollectionCmdRetryTest.java), and :solr:core:check -x test green, with tidy and the Error Prone compile clean. Fork CI runs of all five suites on that same tree are also green.

Round 29 follow-up, verified locally 2026-10-07 at adcda10. The exists guard now has a test, testCreateDoesNotDeleteExistingCollectionOnStaleView in CreateCollectionCleanupTest, which invokes the command with a cluster state that does not contain the collection while ZooKeeper already holds its state.json, and asserts the create fails with BAD_REQUEST and the state.json is byte-identical afterwards. With the exists check disabled in a scratch run, the same test fails because the create's cleanup deletes the pre-existing collection, so the test pins the guard. The failure detail in the core-creation error now names only the first failure instead of dumping the whole failure map into the client message. CreateCollectionCleanupTest 5/5, CreateCollectionCmdRetryTest 4/4, PlacementPluginIntegrationTest 8/8, DeleteCoreRemnantsOnCreateTest 5/5, and :solr:core:check -x test green, with tidy and the Error Prone compile clean. The base re-run was also repeated at c3cdf7b with the tests from this branch: testCleanupAfterPlacementException passes on base, as stated above, and testCleanupAfterUnexpectedPlacementFailure is the one that fails.

Choices to check

Three decisions we made that you may prefer made the other way. Each item ends with what we would change.

1. Alias failure deletes the collection. If the alias write (CreateCollectionCmd.java:500-507) still fails after the bounded retry, the cleanup (CreateCollectionCmd.java:515-516) deletes a collection that was created successfully and is otherwise working, because the create as a whole is reported as failed. One case differs: if an alias attempt was applied and the retries still end in an error, the cleanup's delete is refused because the collection is referenced by an alias, so the collection and the alias both stay and the client gets an error (DeleteCollectionCmd.java:237-244). The alternative is to bound the cleanup at activation, so a failure that late leaves the collection in place and only the alias step is reported. We kept the delete: a create that reports failure should not leave behind a collection the client believes was never created. If you prefer the activation bound, we are willing to make that change in this PR.

2. The exists guard sits in the non-PRS branch only. The deletion risk it closes does not exist in the PRS branch, where the state write (CreateCollectionCmd.java:211-212) throws before the flag is set, so PRS was left as it was. Two inconsistencies remain there: a pre-existing collection missed by the up-front check fails with a 500 carrying the NodeExistsException text instead of the new 400 (CreateCollectionCmd.java:233-236), and a lost reply on the PRS state write (CreateCollectionCmd.java:399-402) can leave a full state.json with no replicas. Moving the guard above the PRS branch split closes both. We shipped it as built to keep this change small; we are willing to do the move in this PR if you prefer the branches to behave the same.

3. Two things left as on main. The cleanup catch (CreateCollectionCmd.java:510) covers Exception, not Error, so an Error still skips the cleanup, as on main. The message for the 30 s wait (Could not fully create collection: <name>, CreateCollectionCmd.java:224 and CreateCollectionCmd.java:259) is unchanged and does not say what timed out. If you want either in this PR, we would catch Throwable for the cleanup and rethrow, and reword that message to name the wait.

Other behavior changes

• Both create timeouts (collection not visible in 30 s (CreateCollectionCmd.java:253-255), replica entries not visible in 120 s (CollectionHandlingUtils.java:346-349)) now delete the collection, and a failing create waits for that delete before returning, which can add up to 60 s (DeleteCollectionCmd.java:149-150) to a failure that used to return at once.
• Smaller changes: on a placement rejection the response no longer carries the delete's output (CreateCollectionCmd.java:574-578); any AssignmentException thrown inside the create, not only one from replica assignment, returns a 400 (CreateCollectionCmd.java:521-524); the thread's interrupt flag is restored where it used to be lost (CreateCollectionCmd.java:511-513); in Overseer mode a create whose up-front check missed an existing collection used to continue against it and now fails with the 400 above (CreateCollectionCmd.java:233-236).
• Outside create, a missing collection now reaches the strategy's other callers as an AssignmentException. CreateShardCmd catches that type and deletes the shard it was creating (CreateShardCmd.java:151-160).

Limits

• An interrupt or shutdown mid-create leaves the collection in place; the cleanup does not run on an interrupted thread (CreateCollectionCmd.java:510-516).
• A failed cleanup leaves the collection, logged at ERROR (CreateCollectionCmd.java:583-587). A cleanup that fails inside DeleteCollectionCmd can leave cores without a collection, because that command still wipes the ZooKeeper tree in its finally block (DeleteCollectionCmd.java:201-212); that matches how a user-issued delete behaves today.
• The alias case under choice 1, where the delete is refused and the collection and the alias both stay.
• Failures before the state write can leave an empty /collections node (CreateCollectionCmd.java:798-799) and an auto-created configset copy (CreateCollectionCmd.java:721-724), as on main.
• The stale re-read window is closed by construction (CreateCollectionCmd.java:269-271) and has no dedicated test; the ZkStateReader window itself is not changed.
• Not covered by a test: the wiring of the alias retry into the create (the unit tests cover the retry helper alone), and the system-collection branch of the strategy guard.
• CreateShardCmd and SplitShardCmd do not get the same treatment. Happy to open a follow-up ticket and PR for them on request.

We limited the scope of this PR on purpose. Several of the points above could be closed here or in a follow-up, and we are glad to do either, so please tell us which of them you want in this PR. The alias retry and the two stale-state workarounds (the cluster-state fill-in and the placement strategy guard) are independently revertible parts of the change; reverting any of them does not affect the cleanup rework.

Changelog: changelog/unreleased/SOLR-18391-collection-creation-failure-cleanup.yml

AI assistance

AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.