[GitHub] [pulsar] aloyszhang commented on a change in pull request #14248: fix rackaware placement policy does not take effect after delete rack configuration

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

aloyszhang commented on a change in pull request #14248:
URL: https://github.com/apache/pulsar/pull/14248#discussion_r805225205



##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/bookie/rackawareness/BookieRackAffinityMapping.java
##########
@@ -201,7 +216,16 @@ private void handleUpdates(Notification n) {
                                 bookieAddressList.add(BookieId.parse(addr));
                             }
                         }
-                        rackawarePolicy.onBookieRackChange(bookieAddressList);
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("Bookies with rack update from {} to {}", bookieAddressListLastTime,
+                                    bookieAddressList);
+                        }
+                        if (bookieAddressListLastTime.size() > bookieAddressList.size()) {
+                            rackawarePolicy.onBookieRackChange(bookieAddressListLastTime);

Review comment:
       This method will be called if there are any changes of content under znode`/bookies`. So, if the contents are exactly the same, this method will not be called.
   
   The change of bookies witch rack has three types: add (add a new bookie with rack), delete (delete an existing bookie with rack), update (update rack for a bookie).
   
   Before this pull request, `handleUpdates` can only deal with add and update, this is because if delete one bookie with rack, the `bookieAddressList` to notify `rackawarePolicy` does not contain the deleted bookie leading to the `NetworkTopology` can't delete the racks of this deleted bookie. And then, this deleted bookie can be still picked into the ensemble.
   
   This pull request resolves this problem, if we delete one bookie with rack, when notifying `rackawarePolicy`, the `bookieAddressList` contains the deleted bookie and all other bookies, so `NetworkTopology` can hear the change(delete) and remove bookie from rack. After this, this deleted bookie can't be selected for  a new ensemble
   




-- 
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: commits-unsubscribe@pulsar.apache.org

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