You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/12/19 11:42:54 UTC

[GitHub] [bookkeeper] TakaHiR07 opened a new pull request, #3707: [improve] replace bookie from different rack of other quorum when do recovery

TakaHiR07 opened a new pull request, #3707:
URL: https://github.com/apache/bookkeeper/pull/3707

   ### Motivation
   
   If we need to do recovery when one bookie down, in the current RackAwarePolicy, it would try picking a candidate from same rack to replace the down bookie firstly. There is a risk that if many bookie in the same rack shutdown, it would result in replacing bookie to the several bookie remain in this rack, making these bookies become hot spot.  
   
   ### Changes
   
   choose the replaced bookie from different rack of other quorum when do recovery or do ensemble change.
   
   
   


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

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


[GitHub] [bookkeeper] hangc0276 commented on pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on PR #3707:
URL: https://github.com/apache/bookkeeper/pull/3707#issuecomment-1358773374

   For `RackAwarePolicy`, it will try its best effort to pick bookies from different racks. When doing recovery, it is safe to replace bookies from the same rack.


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

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


[GitHub] [bookkeeper] TakaHiR07 closed pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by "TakaHiR07 (via GitHub)" <gi...@apache.org>.
TakaHiR07 closed pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery
URL: https://github.com/apache/bookkeeper/pull/3707


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

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


[GitHub] [bookkeeper] TakaHiR07 commented on pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by GitBox <gi...@apache.org>.
TakaHiR07 commented on PR #3707:
URL: https://github.com/apache/bookkeeper/pull/3707#issuecomment-1358828549

   > For `RackAwarePolicy`, it will try its best effort to pick bookies from different racks. When doing recovery, it is safe to replace bookies from the same rack.
   
   In some extreme case, such as 5 bookies in 1 rack, 4 bookies down, 1 bookie survive. The recovery throughput would concentrate on this bookie. After recovery, it would result in data skew in this survived bookie. I think it is not really safe, maybe cause some problem.
   And when do recovery, it seems data locality is not exist. Because the recover bk client in ReplicationWorker is random distribution, it would read data from rack 2 and then recover data to rack 1. So it is not neccessary to ensure the replaced bookie in the same rack


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

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


[GitHub] [bookkeeper] TakaHiR07 commented on pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by GitBox <gi...@apache.org>.
TakaHiR07 commented on PR #3707:
URL: https://github.com/apache/bookkeeper/pull/3707#issuecomment-1359683737

   > > > For `RackAwarePolicy`, it will try its best effort to pick bookies from different racks. When doing recovery, it is safe to replace bookies from the same rack.
   > > 
   > > 
   > > In some extreme case, such as 5 bookies in 1 rack, 4 bookies down, 1 bookie survive. The recovery throughput would concentrate on this bookie. After recovery, it would result in data skew in this survived bookie. I think it is not really safe, maybe cause some problem. And when do recovery, it seems data locality is not exist. Because the recover bk client in ReplicationWorker is random distribution, it would read data from rack 2 and then recover data to rack 1. So it is not neccessary to ensure the replaced bookie in the same rack
   > 
   > @TakaHiR07 If we not ensure the replaced bookie in the same rack, it will break the placement policy in ensemble change and auto recovery.
   
   @hangc0276 This pr only use the implementation in https://github.com/apache/bookkeeper/blob/e6722848e0a599f215e68e1340196c9dfe8e906e/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java#L558-L562  instead of https://github.com/apache/bookkeeper/blob/e6722848e0a599f215e68e1340196c9dfe8e906e/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java#L531-L536
   I think this implementation should not break the RackawarePolicy


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

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


[GitHub] [bookkeeper] hangc0276 commented on pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on PR #3707:
URL: https://github.com/apache/bookkeeper/pull/3707#issuecomment-1360664211

   > > > > For `RackAwarePolicy`, it will try its best effort to pick bookies from different racks. When doing recovery, it is safe to replace bookies from the same rack.
   > > > 
   > > > 
   > > > In some extreme case, such as 5 bookies in 1 rack, 4 bookies down, 1 bookie survive. The recovery throughput would concentrate on this bookie. After recovery, it would result in data skew in this survived bookie. I think it is not really safe, maybe cause some problem. And when do recovery, it seems data locality is not exist. Because the recover bk client in ReplicationWorker is random distribution, it would read data from rack 2 and then recover data to rack 1. So it is not neccessary to ensure the replaced bookie in the same rack
   > > 
   > > 
   > > @TakaHiR07 If we not ensure the replaced bookie in the same rack, it will break the placement policy in ensemble change and auto recovery.
   > 
   > @hangc0276 This pr only use the implementation in
   > 
   > https://github.com/apache/bookkeeper/blob/e6722848e0a599f215e68e1340196c9dfe8e906e/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java#L558-L562
   > 
   > instead of
   > https://github.com/apache/bookkeeper/blob/e6722848e0a599f215e68e1340196c9dfe8e906e/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java#L531-L536
   > 
   > 
   > I think this implementation should not break the RackawarePolicy
   
   @TakaHiR07 The `replaceBookie` will be used in ensemble change and auto recovery. 


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

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


[GitHub] [bookkeeper] hangc0276 commented on pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on PR #3707:
URL: https://github.com/apache/bookkeeper/pull/3707#issuecomment-1359610810

   > > For `RackAwarePolicy`, it will try its best effort to pick bookies from different racks. When doing recovery, it is safe to replace bookies from the same rack.
   > 
   > In some extreme case, such as 5 bookies in 1 rack, 4 bookies down, 1 bookie survive. The recovery throughput would concentrate on this bookie. After recovery, it would result in data skew in this survived bookie. I think it is not really safe, maybe cause some problem. And when do recovery, it seems data locality is not exist. Because the recover bk client in ReplicationWorker is random distribution, it would read data from rack 2 and then recover data to rack 1. So it is not neccessary to ensure the replaced bookie in the same rack
   
   @TakaHiR07 If we not ensure the replaced bookie in the same rack, it will break the placement policy in ensemble change and auto recovery.


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

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


[GitHub] [bookkeeper] StevenLuMT commented on pull request #3707: [improve] replace bookie from different rack of other quorum when do recovery

Posted by GitBox <gi...@apache.org>.
StevenLuMT commented on PR #3707:
URL: https://github.com/apache/bookkeeper/pull/3707#issuecomment-1367759121

   rerun failure checks


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

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