Skip to content

SOLR-18601: Undo the routing rule and buffering when a MIGRATE fails - #5253

Open
SpookyJumpyBeans wants to merge 1 commit into
apache:mainfrom
SpookyJumpyBeans:migrate-cleanup-on-failure
Open

SpookyJumpyBeans wants to merge 1 commit into
apache:mainfrom
SpookyJumpyBeans:migrate-cleanup-on-failure

Conversation

@SpookyJumpyBeans

Copy link
Copy Markdown
Contributor

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

Description

MigrateCmd asks the target leader to buffer updates and adds a routing rule to the source shard before creating a temporary collection, splitting into it and merging into the target. If any of those steps fails, the exception propagates with the target leader still buffering and the routing rule still in place. Updates forwarded under the rule are buffered and never become searchable on the target, and a retry fails immediately because REQUESTBUFFERUPDATES requires the target's update log to be ACTIVE.

Solution

The steps between buffering and applying the buffered updates are wrapped in a try. On failure, cleanupFailedMigrate() removes the routing rule, waits for the removal to be visible (updates forwarded until then would otherwise be indexed by the target once it stops buffering), and then asks the target leader to drop its buffered updates. Errors during cleanup are added to the original exception as suppressed exceptions; an InterruptedException skips cleanup and keeps the interrupt flag.

} catch (Exception e) {
  cleanupFailedMigrate(
      adminCmdContext, sourceCollection, sourceSlice,
      sourceRouter.getRouteKeyNoSuffix(splitKey) + "!", targetLeader, e);
  throw e;
}

Dropping, rather than applying, the buffered updates puts the target back in its pre-MIGRATE state. The documents aren't lost: the source shard leader indexes them and sends them to its own replicas as usual, and forwarding to the MIGRATE target is an additional copy (DistributedZkUpdateProcessor.doDistribAdd/doDistribDeleteById).

There was no core admin action to drop buffered updates (UpdateLog.dropBufferedUpdates() exists but had no callers), so this adds REQUESTDROPBUFFEREDUPDATES, next to REQUESTBUFFERUPDATES and REQUESTAPPLYUPDATES and modeled on them (missing core is a 400, not buffering is a 500, unexpected errors are wrapped in a 500).

Notes:

  • Most of the MigrateCmd diff is re-indentation for the new try block; it's much smaller with whitespace ignored.
  • The new action is internal and v1 only, like how MIGRATE uses its two siblings. They also have v2 wrappers; I didn't add one for this action, but can if that's wanted.
  • A failure of the final REQUESTAPPLYUPDATES itself isn't covered: by then the target has the merged index, and dropping the buffer would lose updates made during the MIGRATE.
  • An update that resolved the routing rule just before it was removed could still reach the target after the buffer is dropped and be indexed there. Waiting for the rule removal makes this window small but doesn't close it.
  • If the merge actually completed but its response was lost, the target already has the merged documents; the cleanup can't tell this apart from a failed merge.
  • The temporary collection is left behind on failure, as before. The next MIGRATE already deletes a leftover temporary collection before it starts.
  • Unchanged: an async MIGRATE doesn't abort when a core admin request fails (ShardRequestTracker.processResponses only records the failure), so some failures still let it run to the end.

Tests

Added MigrateRouteKeyTest.testRetryAfterFailedMigrate. It makes MIGRATE fail after the target leader started buffering: an alias refers to a leftover temporary collection, so MIGRATE can't delete it and then fails to create it. It does this once synchronously and once asynchronously, and after each checks that the target leader's update log is ACTIVE and the routing rule is gone. It then removes the alias, retries the MIGRATE, and checks that the target has exactly the migrated documents.

Without the fix, both the synchronous and the asynchronous failure leave the target leader in BUFFERING (expected:<ACTIVE> but was:<BUFFERING>), and a retry fails with "MIGRATE failed to request node to buffer updates". With the fix, MigrateRouteKeyTest passes on 3 runs with different seeds. Also added tests for the new action's error handling to CoreAdminOperationTest. CoreAdminOperationTest, CoreAdminHandlerTest and V2CoreAPIMappingTest pass, and so does ./gradlew :solr:solrj:check :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

MIGRATE asks the target leader to buffer updates and adds a routing rule
to the source shard before the steps that can fail (creating the
temporary collection, splitting, adding a replica, merging). A failure
in any of them left the target leader buffering, so forwarded updates
were never searchable on the target and a retry failed immediately.

On such a failure, remove the routing rule, wait for the removal, and
drop the target leader's buffered updates. This adds an internal
REQUESTDROPBUFFEREDUPDATES core admin action, next to
REQUESTBUFFERUPDATES and REQUESTAPPLYUPDATES.
SpookyJumpyBeans added a commit to SpookyJumpyBeans/boyidai that referenced this pull request Oct 28, 2026
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.

1 participant