[GitHub] philipsu522 commented on a change in pull request #776: Issue 709: Add Slow Bookkeeper Servers to Placement Policy for read ordering

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

philipsu522 commented on a change in pull request #776: Issue 709: Add Slow Bookkeeper Servers to Placement Policy for read ordering
URL: https://github.com/apache/bookkeeper/pull/776#discussion_r155149682
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java
 ##########
 @@ -779,37 +796,107 @@ protected BookieNode selectRandomFromRack(String netPath, Set<Node> excludeBooki
         throw new BKNotEnoughBookiesException();
     }
 
+    @Override
+    public void registerSlowBookie(BookieSocketAddress bookieSocketAddress, long entryId) {
+        slowBookies.put(bookieSocketAddress, entryId);
+
+    }
+
     @Override
     public DistributionSchedule.WriteSet reorderReadSequence(
             ArrayList<BookieSocketAddress> ensemble,
-            Map<BookieSocketAddress, Long> bookieFailureHistory,
+            BookKeeperServerHealthInfo bookKeeperServerHealthInfo,
             DistributionSchedule.WriteSet writeSet) {
+        Map<Integer, String> writeSetWithRegion = new HashMap<>();
+        for (int i = 0; i < writeSet.size(); i++) {
+            writeSetWithRegion.put(writeSet.get(i), "");
+        }
+        return reorderReadSequenceWithRegion(ensemble, writeSet, writeSetWithRegion, bookKeeperServerHealthInfo, false, "", writeSet.size());
+
+    }
+
+    /**
+     * This function orders the read sequence with a given region. For region-unaware policies (e.g.
+     * RackAware), we pass in false for regionAware and an empty myRegion. When this happens, any
+     * remote list will stay empty. The ordering is as follows (the R* at the beginning of each list item
+     * is only present for region aware policies.
+     *      1. available (local) bookies
+     *      2. R* a remote bookie (based on remoteNodeInReorderSequence
+     *      3. R* remaining (local) bookies
+     *      4. R* remaining remote bookies
+     *      5. read only bookies
+     *      6. slow bookies
+     *      7. unavailable bookies
+     *
+     * @param ensemble
+     *          ensemble of bookies
+     * @param writeSet
+     *          write set
+     * @param writeSetWithRegion
+     *          write set with region information
+     * @param bookKeeperServerHealthInfo
+     *          heuristics about health of boookies
+     * @param regionAware
+     *          whether or not a region-aware policy is used
+     * @param myRegion
+     *          current region of policy
+     * @param remoteNodeInReorderSequence
+     *          number of local bookies to try before trying a remote bookie
+     * @return ordering of bookies to send read to
+     */
+    DistributionSchedule.WriteSet reorderReadSequenceWithRegion(
+        ArrayList<BookieSocketAddress> ensemble,
+        DistributionSchedule.WriteSet writeSet,
+        Map<Integer, String> writeSetWithRegion,
+        BookKeeperServerHealthInfo bookKeeperServerHealthInfo,
+        boolean regionAware,
+        String myRegion,
+        int remoteNodeInReorderSequence) {
+        boolean useRegionAware = regionAware && (!myRegion.equals(UNKNOWN_REGION));
         int ensembleSize = ensemble.size();
 
         for (int i = 0; i < writeSet.size(); i++) {
-            int idx = writeSet.get(i);
+            Integer idx = writeSet.get(i);
             BookieSocketAddress address = ensemble.get(idx);
-            Long lastFailedEntryOnBookie = bookieFailureHistory.get(address);
+            String region = writeSetWithRegion.get(idx);
+            Long lastFailedEntryOnBookie = bookKeeperServerHealthInfo.getBookieFailureHistory(address);
             if (null == knownBookies.get(address)) {
                 // there isn't too much differences between readonly bookies
                 // from unavailable bookies. since there
                 // is no write requests to them, so we shouldn't try reading
-                // from readonly bookie in prior to writable
-                // bookies.
+                // from readonly bookie prior to writable bookies.
                 if ((null == readOnlyBookies)
-                        || !readOnlyBookies.contains(address)) {
+                    || !readOnlyBookies.contains(address)) {
                     writeSet.set(i, idx | UNAVAIL_MASK);
                 } else {
-                    writeSet.set(i, idx | READ_ONLY_MASK);
+                    if (slowBookies.asMap().containsKey(address)) {
 
 Review comment:
   fixed in latest commit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services