kirklund commented on a change in pull request #6909:
URL: https://github.com/apache/geode/pull/6909#discussion_r720525647
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java
##########
@@ -1712,8 +1712,7 @@ private PartitionedRegion
findPersistentRegionRecursively(PartitionedRegion part
}
void scheduleCreateMissingBuckets() {
- if (partitionedRegion.getColocatedWith() != null
- && ColocationHelper.isColocationComplete(partitionedRegion)) {
+ if (partitionedRegion.getColocatedWith() != null) {
Review comment:
PRHARedundancyProviderTest should be updated with a test that fails
without this change.
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/CreateMissingBucketsTask.java
##########
@@ -18,18 +18,30 @@
import org.apache.geode.internal.cache.PRHARedundancyProvider;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.cache.PartitionedRegion.RecoveryLock;
+import org.apache.geode.internal.cache.PartitionedRegionHelper;
/**
* A task for creating buckets in a child colocated region that are present in
the leader region.
*
*/
public class CreateMissingBucketsTask extends RecoveryRunnable {
+ private static final int MAX_NUMBER_INTERVALS = 31;
+
+ private static final int SMALL_INTERVALS = 5;
+ private static final int MEDIUM_INTERVALS = 10;
+ private static final int LARGE_INTERVALS = 15;
+
public CreateMissingBucketsTask(PRHARedundancyProvider
prhaRedundancyProvider) {
super(prhaRedundancyProvider);
}
@Override
public void run2() {
+ if
(!waitForColocationCompleted(redundancyProvider.getPartitionedRegion())) {
+ // if after all the time, colocation is still not completed, do nothing
+ return;
+ }
+
Review comment:
Let's create a `CreateMissingBucketsTaskTest` (unit test) that tests
this change. `ColocationHelper` makes the testing more difficult, but you can
change `CreateMissingBucketsTask` to wrap the two static calls with
`java.util.function.Function`s:
```
public class CreateMissingBucketsTask extends RecoveryRunnable {
private final Function<PartitionedRegion, PartitionedRegion>
getLeaderRegionFunction;
private final Function<PartitionedRegion, PartitionedRegion>
getColocatedRegionFunction;
public CreateMissingBucketsTask(PRHARedundancyProvider
prhaRedundancyProvider) {
this(prhaRedundancyProvider,
ColocationHelper::getLeaderRegion,
ColocationHelper::getColocatedRegion);
}
CreateMissingBucketsTask(PRHARedundancyProvider prhaRedundancyProvider,
Function<PartitionedRegion, PartitionedRegion>
getLeaderRegionFunction,
Function<PartitionedRegion, PartitionedRegion>
getColocatedRegionFunction) {
super(prhaRedundancyProvider);
this.getLeaderRegionFunction = getLeaderRegionFunction;
this.getColocatedRegionFunction = getColocatedRegionFunction;
}
@Override
public void run2() {
// PartitionedRegion leaderRegion =
//
ColocationHelper.getLeaderRegion(redundancyProvider.getPartitionedRegion());
PartitionedRegion leaderRegion =
getLeaderRegionFunction.apply(redundancyProvider.getPartitionedRegion());
RecoveryLock lock = leaderRegion.getRecoveryLock();
lock.lock();
try {
createMissingBuckets(redundancyProvider.getPartitionedRegion());
} finally {
lock.unlock();
}
}
protected void createMissingBuckets(PartitionedRegion region) {
// PartitionedRegion parentRegion =
ColocationHelper.getColocatedRegion(region);
PartitionedRegion parentRegion =
getColocatedRegionFunction.apply(region);
if (parentRegion == null) {
return;
}
// Make sure the parent region has created missing buckets
// before we create missing buckets for this child region.
createMissingBuckets(parentRegion);
for (int i = 0; i < region.getTotalNumberOfBuckets(); i++) {
if
(parentRegion.getRegionAdvisor().getBucketAdvisor(i).getBucketRedundancy() !=
region
.getRegionAdvisor().getBucketAdvisor(i).getBucketRedundancy()) {
region.getRedundancyProvider().createBucketAtomically(i, 0, true,
null);
}
}
}
}
```
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/CreateMissingBucketsTask.java
##########
@@ -58,4 +70,38 @@ protected void createMissingBuckets(PartitionedRegion
region) {
}
}
}
+
+ /**
+ * Wait for Colocation to complete. Wait all nodes to Register this
PartitionedRegion.
+ */
+ private boolean waitForColocationCompleted(PartitionedRegion
partitionedRegion) {
+ int retryCount = 0;
+ int sleepInterval =
PartitionedRegionHelper.DEFAULT_WAIT_PER_RETRY_ITERATION;
+
+ while (!ColocationHelper.isColocationComplete(partitionedRegion)
Review comment:
`ColocationHelper.isColocationComplete` will need to wrapped in a
`java.util.function.Function` as well to control this for unit testing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
