ctubbsii commented on PR #3479:
URL: https://github.com/apache/accumulo/pull/3479#issuecomment-1585224599

   > @ctubbsii - Specifically `destinations` is passed to `flushChanges` at 
https://github.com/apache/accumulo/blob/1.10/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java#LL214C26-L214C38,
 not the current set of tservers.
   
   Okay, I got it now. It looks like the mistake was in the change to 
`flushChanges` in #1761. The parameter name was `currentTServers`, but it was 
passed `destinations`. But, outside that method, there was a `currentTServers` 
in the `run` method. It looks like the parameter name for `flushChanges` got 
misunderstood to be the same as the `currentTServers` variable, when it never 
actually was... it was always `destinations` by the wrong name.
   
   Okay, after reviewing the details, I'm confident this fixes the issue in 
2.1.0. However, I'd like to keep the original `destinations` name, rather than 
rename the variable. It just makes the already complicated code history easier 
to follow when we're not arbitrarily changing variable names alongside the 
substantive changes. And, in this case, I don't think adding "valid" to the 
variable name makes it more clear.


-- 
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]

Reply via email to