SOLR-18382: remove DocCollection.getReplicas(), migrate 60 call sites - #4760
Open
serhiy-bzhezytskyy wants to merge 1 commit into
Open
Conversation
The method flattened every slice's replicas into a fresh ArrayList on each
call. DocCollection is already Iterable<Slice>, so callers iterate slices,
or use the idiom the tree already had in four places:
X.getSlices().stream().flatMap(slice -> slice.getReplicas().stream())
The census had to come from the compiler, not from grep. `git grep
'\.getReplicas()'` returns 285 hits and only 60 are this method: the name
is declared seven times in the tree, and Slice.getReplicas() accounts for
most of the rest. No textual pattern can separate them, because the
receiver's name carries no type - the 60 real sites are reached through
eleven different variable names including `slices`, which is a
DocCollection named as if it were a Slice. So the method was deleted first
and the error list became the worklist.
And a failing build is not a complete census. The first compile reported
2 call sites; the true number was 60. It had died in test-framework, so
core's test compilation never ran. The list is the fixed point of
delete-compile-fix, not the first report.
Split by source set, because the two halves answer different questions:
production src/java 2 sites - the whole compatibility surface
test-framework/src/java 3 sites - a published artifact, so this
counts as API
src/test 55 sites in 32 files
That makes the deprecation note - "low usage and builds an ArrayList
(surprising)" - correct about what it cares about. Two production callers
is low usage. It is the test migration that makes the ticket large.
Three things the type change could have broken, all checked. The removed
method returned List<Replica> while Slice.getReplicas() returns
Collection<Replica>, so sites needing indexed access collect to a real
list; the three .toList() sites are read-only, including the one passed
to assertDocsExistInAllReplicas, whose two overloads only iterate their
argument. Traversal order is unchanged, so sites doing .get(0) or
iterator().next() still pick the same replica; nothing was sorted. No
asserted value was altered - every expected literal (1, 2, 4, 8, 8, 9)
and every assertion message survives verbatim, and of 147 removed lines,
77 reappear identically modulo indentation from the added nesting.
One conversion needed care: turning a single loop into a nested pair
inside a lambda collided with an enclosing local named `slice` in
CollectionTooManyReplicasTest, which Java forbids; the loop variable is
`s` there.
Verified: compileJava and compileTestJava for the whole build,
spotlessCheck, ecjLintMain on core, solrj and test-framework, and all 32
changed test classes - 413 tests, 0 failures, 70 skipped, with every
class confirmed to have produced a result file rather than being
silently filtered out.
AI-assisted (Claude Sonnet 5)
github-actions
Bot
added
test-framework
jetty-server
client:solrj
tests
cat:search
cat:cloud
cat:index
cat:api
labels
This was referenced Aug 19, 2026
Contributor
Author
|
@dsmiley this one's yours ( AI-assisted (Claude Sonnet 5) |
dsmiley
self-requested a review
August 20, 2026 00:16
dsmiley
approved these changes
Aug 20, 2026
dsmiley
left a comment
dsmiley
left a comment
Contributor
There was a problem hiding this comment.
Thanks.
Admittedly it adds more code in many places. Maybe a getReplicaStream() method would be useful?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-18382
Removes
DocCollection.getReplicas()(flattened every slice into a freshArrayListon each call) and migrates 60 call sites — the real count, not the 285 a plain.getReplicas()grep returns, sinceSlice.getReplicas()shares the name. The method was deleted first and the compiler's error list became the worklist.Split by source set: 2 production sites, 3 in
test-framework(a published artifact), 55 across 32 test files — that's why this ticket is large despite "low usage" being correct about production.Where to look:
CollectionTooManyReplicasTest.javahas a local namedslicealready in scope at one call site, so the migrated loop there usessinstead — everywhere else usesslice. Three checks before trusting the swap: traversal order is unchanged (nothing was sorted), the three.toList()sites are read-only, and every asserted literal survives verbatim.413 tests across 32 changed classes, 0 failures. Compile is the actual census here — a failing build's first report showed 2 sites; the true number surfaced only once test-framework compiled too.
SOLR-18378, SOLR-18380, SOLR-18381 and SOLR-18385 touch files this PR also touches — merging this one first should make those cleaner to extract.
AI-assisted (Claude Sonnet 5)