[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2642: fix region/rack aware placement police replace bookie bug

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #2642:
URL: https://github.com/apache/bookkeeper/pull/2642#discussion_r592955861



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RegionAwareEnsemblePlacementPolicy.java
##########
@@ -446,7 +447,10 @@ public RegionAwareEnsemblePlacementPolicy initialize(ClientConfiguration conf,
 
             BookieNode bookieNodeToReplace = knownBookies.get(bookieToReplace);
             if (null == bookieNodeToReplace) {
-                bookieNodeToReplace = createBookieNode(bookieToReplace);
+                bookieNodeToReplace = historyBookies.get(bookieToReplace);

Review comment:
       the same here

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java
##########
@@ -456,7 +456,10 @@ public void uninitalize() {
             excludeBookies.addAll(currentEnsemble);
             BookieNode bn = knownBookies.get(bookieToReplace);
             if (null == bn) {
-                bn = createBookieNode(bookieToReplace);
+                bn = historyBookies.get(bookieToReplace);
+                if (null == bn) {
+                    bn = createBookieNode(bookieToReplace);

Review comment:
       probably it is better to try to use createBookieNode and fallback to history if it fails.
   otherwise you won't be able to follow a change in the network location for the bookie.




----------------------------------------------------------------
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:
users@infra.apache.org