[GitHub] [geode] upthewaterspout commented on a diff in pull request #7630: GEODE-10242: Do not release primary lock prematurely

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

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

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