upthewaterspout commented on code in PR #7630:
URL: https://github.com/apache/geode/pull/7630#discussion_r860227852
##########
geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java:
##########
@@ -896,9 +888,10 @@ private void removePrimary(InternalDistributedMember
member) {
((BucketRegion) br).beforeReleasingPrimaryLockDuringDemotion();
}
- releasePrimaryLock();
// this was a deposePrimary call so we need to depose children as well
deposePrimaryForColocatedChildren();
+ releasePrimaryLock();
Review Comment:
Do we need to make this change? This is releasing the dlock after all
children are deposed as primary. I thought just the fact that we are holding
the primary move write lock this whole time will prevent any operations from
being in progress on any child/grandchild region while this depose is going on.
##########
geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java:
##########
@@ -1670,7 +1663,7 @@ protected Set<BucketServerLocation66>
getBucketServerLocations(int version) {
*
* @param id the member to use as primary for this bucket
*/
- private void setPrimaryMember(InternalDistributedMember id) {
+ synchronized void setPrimaryMember(InternalDistributedMember id) {
Review Comment:
Why did we add synchronized here?
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
