clintropolis commented on code in PR #20267:
URL: https://github.com/apache/druid/pull/20267#discussion_r3962119887
##########
server/src/main/java/org/apache/druid/server/coordinator/loading/StrategicSegmentAssigner.java:
##########
@@ -516,25 +520,68 @@ private int loadPartialReplicas(
return 0;
}
- // The classifier's list is already the complete candidate set when
nothing was canceled.
+ // The classifier's lists are already the complete candidate sets when
nothing was canceled.
+ final List<ServerHolder> inPlaceDestinations;
final List<ServerHolder> freshCandidates;
if (canceledStaleServers.isEmpty()) {
+ inPlaceDestinations = status.getEligibleForInPlaceReload();
freshCandidates = status.getEligibleForFreshLoad();
} else {
+ inPlaceDestinations = new
ArrayList<>(status.getEligibleForInPlaceReload());
freshCandidates = new ArrayList<>(status.getEligibleForFreshLoad());
- freshCandidates.addAll(canceledStaleServers);
+ for (ServerHolder server : canceledStaleServers) {
+ if (server.isServingSegment(segment) &&
PartialSegmentStatusInTier.canReloadInPlace(server)) {
+ inPlaceDestinations.add(server);
+ } else {
+ freshCandidates.add(server);
+ }
+ }
}
- final Iterator<ServerHolder> destinations = Iterators.concat(
- serversToLoadSegment(segment, tier, freshCandidates),
- status.getEligibleForAdditiveReload().iterator()
+ int numLoadsQueued = queuePartialLoads(
Review Comment:
i think not dropping and retrying the in-place reload is the best way to
handle this, so i've made that change.
i think the only 'expected' way for the reload to fail is if there is
insufficient space; where leaving the stale replica there and retrying on the
next run seems fine. If there is sufficient room on other servers, I would
expect balancing to move some segments from the server with the stales, in
which case the retry works after balancing, or else the other nodes are also
full, in which case everything is going to be 'stuck' and so also not a problem
if we keep retrying in-place here. Any other cause (transient problem, etc)
should work on the retry, or if not, is something that would also be a problem
on fresh load (like missing from deep storage or whatever).
##########
server/src/main/java/org/apache/druid/server/coordinator/loading/StrategicSegmentAssigner.java:
##########
@@ -516,25 +520,68 @@ private int loadPartialReplicas(
return 0;
}
- // The classifier's list is already the complete candidate set when
nothing was canceled.
+ // The classifier's lists are already the complete candidate sets when
nothing was canceled.
+ final List<ServerHolder> inPlaceDestinations;
final List<ServerHolder> freshCandidates;
if (canceledStaleServers.isEmpty()) {
+ inPlaceDestinations = status.getEligibleForInPlaceReload();
freshCandidates = status.getEligibleForFreshLoad();
} else {
+ inPlaceDestinations = new
ArrayList<>(status.getEligibleForInPlaceReload());
freshCandidates = new ArrayList<>(status.getEligibleForFreshLoad());
- freshCandidates.addAll(canceledStaleServers);
+ for (ServerHolder server : canceledStaleServers) {
+ if (server.isServingSegment(segment) &&
PartialSegmentStatusInTier.canReloadInPlace(server)) {
+ inPlaceDestinations.add(server);
+ } else {
+ freshCandidates.add(server);
+ }
+ }
}
- final Iterator<ServerHolder> destinations = Iterators.concat(
- serversToLoadSegment(segment, tier, freshCandidates),
- status.getEligibleForAdditiveReload().iterator()
+ int numLoadsQueued = queuePartialLoads(
Review Comment:
i think not dropping and retrying the in-place reload is the best way to
handle this, so i've made that change.
i think the only 'expected' way for the reload to fail is if there is
insufficient space; where leaving the stale replica there and retrying on the
next run seems fine. If there is sufficient room on other servers, I would
expect balancing to move some segments from the server with the stales, in
which case the retry works after balancing, or else the other nodes are also
full, in which case everything is going to be 'stuck' and so also not a problem
if we keep retrying in-place here.
Any other cause (transient problem, etc) should work on the retry, or if
not, is something that would also be a problem on fresh load (like missing from
deep storage or whatever).
--
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]
