[GitHub] [bookkeeper] dlg99 commented on a diff in pull request #3354: Resolve network location if the bookie is down but history holds it

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

dlg99 commented on code in PR #3354:
URL: https://github.com/apache/bookkeeper/pull/3354#discussion_r905357515


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java:
##########
@@ -1026,7 +1026,14 @@ public PlacementPolicyAdherence isEnsembleAdheringToPlacementPolicy(List<BookieI
             for (int j = 0; j < writeQuorumSize; j++) {
                 bookie = ensembleList.get((i + j) % ensembleSize);
                 try {
-                    racksInQuorum.add(knownBookies.get(bookie).getNetworkLocation());
+                    BookieNode bookieNode = knownBookies.get(bookie);
+                    if (bookieNode == null) {
+                        bookieNode = historyBookies.get(bookie);

Review Comment:
   now the behavior depends on whether the bookie ran for awhile and hs history or freshly restarted.
   
   AFAICT this should affect Auditor which does isEnsembleAdheringToPlacementPolicy but I guess our test coverage for the rackaware policy + bookie removal/Auditor runs is not good enough.
   
   i think the only case where the historyBookies is to protect from DNS resolver errors, this changes expectations. 
   
   



-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org