nick-boss-tech commented on PR #5265: URL: https://github.com/apache/solr/pull/5265#issuecomment-6598957274
🤖 *AI text below* 🤖 *(reviewed by Nick Shanin)* Thanks, Eric. This is wired now, at 7a020053d1. `CollectionsHandler.SPLITSHARD_OP` delegates as you sketched: it calls `SplitShard.invokeWithV1Params(...)`, which builds the request body through `createRequestBodyFromV1Params` and runs the resource; the operation entry keeps its 5x timeout. The duplicated cross-parameter checks are gone from the handler, and `SplitShard.validateSplitOptions()` is the single copy. One deviation from your note, with the evidence for it. You suggested leaving the body's `async` field unset, since v1's dispatch wraps the call in its own async-task submission. That holds for the old shape, where the operation returns a message map and `CollectionsHandler.invokeAction` submits it with the v1 async id in the command context. A delegated operation returns null there, and the handler submits nothing, so the resource's submission has to carry the id, the way the `CreateShard` and `DeleteShard` adapters already do it. The nightly `ShardSplitTest` shows the difference: with `async` unset, the static-index split tests fail in the SolrJ client because `processAsync` gets no request id back; with the v1 `async` parameter mapped into the body, they get past that point. The conversion maps it, and a unit test pins the mapped id. Two v1 shapes came up while wiring it, and I kept both working rather than dropping them quietly: - Split by `split.key` with no shard named. The v1 handler allows it, and SolrJ's `CollectionAdminRequest.SplitShard` requires only one of shard or split.key; the overseer resolves the shard from the key. The resource cannot express it, since shard is a path parameter, so for that shape the adapter submits through the resource class with no shard entry in the message, after the same validation. The shard/split.key presence and mutual-exclusion checks live in the adapter, since they are about the v1 parameter shape. - `createNodeSet`. The v1 handler forwarded it and subshard placement reads it, but the request body had no field for it. I added `createNodeSet` to `SplitShardRequestBody`, so it reaches the overseer message on both routes; v2 callers gain it as well. <details> <summary>Verification at 7a020053d19435e29036d96d2741ce890cef75c6</summary> Tidy clean; Error Prone compile passes; `:solr:core:check -x test` passes. SplitShardAPITest 5 tests, SplitShardMessageMappingTest 19 tests (five new ones pin the v1 adapter: the parameter conversion including the async id, the shard-path delegation message, the split-by-key message with no shard entry, and both v1 shape rejections), V2CollectionAPIMappingTest 4, TestCollectionAPIs 2, TestApiFramework 3, all passing at seed 16398C0DE. ShardSplitTest with the nightly group enabled: 11 tests, 1 intermittent failure in testSplitStaticIndexReplication's post-restart replica-consistency check; the same check fails intermittently at the pre-change base under the same seed and flags, on both variants, so it is not an effect of this change. </details> If you would rather the split-by-key shape not be carried by the adapter, I can take that part out; breaking SolrJ's split-by-key path silently seemed worse. -- 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]
