ctubbsii commented on a change in pull request #1653:
URL: https://github.com/apache/accumulo/pull/1653#discussion_r487200764
##########
File path:
server/manager/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
##########
@@ -871,7 +870,9 @@ private void
flushChanges(SortedMap<TServerInstance,TabletServerStatus> currentT
if (!assignments.isEmpty()) {
Master.log.info(String.format("Assigning %d tablets",
assignments.size()));
- store.setFutureLocations(assignments);
+
+ for (Assignment assignment : assignments)
+ store.setFutureLocation(assignment);
Review comment:
Based on the code you changed, it does not appear that's the case.
Specifically, where you made this update in a loop, it wasn't one at a time. I
think you could probably have both the single-case version, and the collection
version, so we don't have to do "Collections.singletonList" everywhere when we
only have one. The Collection one could be optimized to reuse the same
batchwriter somehow (not sure exactly what would need to change in the tablet
mutator / Ample code to do that; the old code that didn't use Ample was
obviously reusing a batch writer, but I'm not sure how to do it in an Ample
way). I'm also not sure how you'd pass the previous location to clear if you're
passing a collection... you might have a different previous location for each
assignment, right? I'm not sure without analyzing the code in a lot more detail.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
