nick-boss-tech opened a new pull request, #5088: URL: https://github.com/apache/solr/pull/5088
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-13696 ## What happens today **The Category and Dimensional routed-alias update tests are skipped. When the skip is removed, they fail in several places.** Both suites extend `RoutedAliasUpdateProcessorTest`, which was skipped for this ticket. When the skip is removed: - The create-alias message for a dimensional alias has no top-level router name or field. The Dimensional suite cannot create its alias. - The Dimensional tests use a fixed "future" date, 2020-10-23. That date is now in the past. The router then creates daily collections toward it until the client times out. - The final placement check reads `[shard]` in its default style. At luceneMatchVersion 9.5 that style returns a bare shard id. The id holds neither the category nor the day the check asserts. - A document whose timestamp is still a string at update-processor time throws `ClassCastException` in `TimeRoutedAlias.formattedRouteValues`. The create-alias failure stops the suite before it gets that far. - Each test method creates its own cluster, but only the last one is shut down. The earlier clusters leak and cause teardown failures. ## What this change does **The two suites run again, their tests are repaired, and two production defects are fixed.** - The `@AwaitsFix` on the base class is removed, so the Category and Dimensional suites run in normal mode. The base class keeps its `@Ignore`, since it is abstract ([base class](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java#L67)). - `addDocsAndCommit` commits each collection of the alias explicitly and no longer relies on commitWithin ([commit loop](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java#L360-L370)). - Each test method shuts down its own cluster in an `@After` method ([L72-L82](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java#L72-L82)). - The category test now uses `ship_name_s` instead of `ship_name_en` ([L53](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java#L53)). - The "future" document is built 30 days after the current time ([helper](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java#L615-L621)). The placement check asks for `[shard]` in URL style ([L282-L293](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java#L282-L293)). - Create-alias fix: for a dimensional alias, the message now carries the top-level router name, in the `Dimensional[TIME,CATEGORY]` form, and the comma-joined router field. That is the shape SolrJ already sends ([CreateAlias](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/java/org/apache/solr/handler/admin/api/CreateAlias.java#L128-L141), [SolrJ](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java#L2326-L2327)). The change applies to every dimensional alias created through the create-alias API, not only in tests. `CreateAliasAPITest` now expects 12 entries, up from 10, and checks both new keys ([L308-L311](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/handler/admin/api/CreateAliasAPITest.java#L308-L311)). - Time-route fix: `formattedRouteValues` parses the value with `parseRouteKey` instead of casting it to `Date` ([L219-L225](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java#L219-L225), [parseRouteKey](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java#L468-L479)). A string timestamp is now parsed. Only dimensional aliases call this method ([caller](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/java/org/apache/solr/cloud/api/collections/DimensionalRoutedAlias.java#L120-L124)). ## Proof **The focused suites pass at head `da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9`, and each repair has a pre-fix run on record.** - `CategoryRoutedAliasUpdateProcessorTest` 6 of 6. - `DimensionalRoutedAliasUpdateProcessorTest` 2 of 2. - `CreateAliasAPITest` 13 of 13. These counts come from fresh JUnit XML at this head. - Time-route fix: on pre-fix production head `98ad9d3fcc33` (seed `FB1F0CEBAAF65F30`), both Dimensional tests fail with `ClassCastException`. - Future-date repair: at `a4e0da422327` (seed `1676C9C3B647F0E6`), the test file from before that repair fails with the timeout shape. - The `[shard]` repair: at `08f9384e47c0` (seed `9DBC31B7C732B317`), the test file from before that repair fails at the final placement check, in the final-loop assertion shape. - Tidy returns 0 with a clean tree. The Error Prone compile returns 0. `:solr:core:check -x test` returns 0. ## A choice to check **Two production fixes ride along with the test repair. Each could move to its own ticket and PR.** **The create-alias message.** Options considered: - Implemented: the fix stays in this branch. The Dimensional suite needs it to create its alias. - Alternative: a separate ticket and PR. The Dimensional suite stays broken until that PR lands. Was keeping the create-alias fix in this branch the right call? **The time-route cast.** Options considered: - Implemented: the fix stays in this branch. The cast defect sat behind the create-alias defect, so the suite reaches it only after that fix. - Alternative: a separate ticket and PR. Was keeping the time-route fix in this branch the right call? ## Limits **This change adds no Time coverage, and the routed-alias tests no longer use commitWithin.** - `TimeRoutedAliasUpdateProcessorTest` keeps its own `@AwaitsFix(SOLR-13059)` ([L75](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java#L75)) and stays skipped. No Time coverage comes from this change. One more method in that class, `testDateMathInStart`, is skipped under SOLR-13943 ([L966](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java#L966)), which is separate work. - The create-alias fail-before is the investigation run on the untouched base `c3cdf7b46e8`, with the awaitsfix group enabled (seed `54689CC480DC14B0`). Both Dimensional tests fail at CREATEALIAS with "requires these params: [router.name, router.field]". - Every document is committed explicitly, so commitWithin is no longer exercised. A follow-up ticket and PR can restore that coverage on request. - Only the Dimensional suite reaches the time-route change, through its time dimension. Its documents send the timestamp as a string ([newDoc](https://github.com/nick-boss-tech/solr/blob/da4fa6df11784ce5a83bc7e74ec7b6aa78f689b9/solr/core/src/test/org/apache/solr/update/processor/DimensionalRoutedAliasUpdateProcessorTest.java#L644-L650)). A single time alias does not call this method, so the change has no effect there. Changelog: `changelog/unreleased/SOLR-13696-dimensional-routed-alias.yml` (type `fixed`), one entry covering both production fixes. ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- 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]
