[
https://issues.apache.org/jira/browse/GEODE-8339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17193726#comment-17193726
]
ASF GitHub Bot commented on GEODE-8339:
---------------------------------------
dschneider-pivotal commented on a change in pull request #5501:
URL: https://github.com/apache/geode/pull/5501#discussion_r486491077
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -622,7 +623,9 @@ public void computeWithPrimaryLocked(Object key, Runnable
r) throws PrimaryBucke
}
try {
- br.doLockForPrimary(false);
+ if (!br.doLockForPrimary(false)) {
Review comment:
remove the "if" and just call br.doLockForPrimary(false);
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -611,7 +611,8 @@ PartitionedRegionRedundancyTracker getRedundancyTracker() {
return redundancyTracker;
}
- public void computeWithPrimaryLocked(Object key, Runnable r) throws
PrimaryBucketLockException {
+ public boolean computeWithPrimaryLocked(Object key, Runnable r)
Review comment:
this should no longer return boolean (since we got rid of the tryLock
option)
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PrimaryBucketLockException.java
##########
@@ -19,7 +19,7 @@
import org.apache.geode.GemFireException;
/**
- * Thrown by {@link PartitionedRegion#computeWithPrimaryLocked(Object,
Runnable)}
+ * Thrown by {@link PartitionedRegion#computeWithPrimaryLocked(Object,
Runnable, boolean)}
Review comment:
this javadoc needs to be updated for the signature with no boolean
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -3395,11 +3399,10 @@ private InternalDistributedMember
getNodeForBucketReadOrLoad(int bucketId) {
* @throws PrimaryBucketException if the remote bucket was not the primary
* @throws ForceReattemptException if the peer is no longer available
*/
+
public boolean putRemotely(final DistributedMember recipient, final
EntryEventImpl event,
boolean ifNew, boolean ifOld, Object expectedOldValue, boolean
requireOldValue)
throws PrimaryBucketException, ForceReattemptException {
- // boolean forceAck = basicGetWriter() != null
- // || getDistributionAdvisor().adviseNetWrite().size() > 0;
Review comment:
restore this comment; basically get this PR to the point that it does
not modify PartitionedRegion.java
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
##########
@@ -5718,7 +5718,6 @@ RegionEntry basicPutEntry(final EntryEventImpl event,
final long lastModified)
@Override
public long basicPutPart2(EntryEventImpl event, RegionEntry entry, boolean
isInitialized,
long lastModified, boolean clearConflict) {
-
Review comment:
put this blank line back so that this PR does not modify LocalRegion.java
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -3395,11 +3399,10 @@ private InternalDistributedMember
getNodeForBucketReadOrLoad(int bucketId) {
* @throws PrimaryBucketException if the remote bucket was not the primary
* @throws ForceReattemptException if the peer is no longer available
*/
+
Review comment:
take out this new blank line
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Redis rename hangs when servers are killed and revived
> ------------------------------------------------------
>
> Key: GEODE-8339
> URL: https://issues.apache.org/jira/browse/GEODE-8339
> Project: Geode
> Issue Type: Bug
> Components: redis
> Reporter: Sarah Abbey
> Priority: Major
> Labels: pull-request-available
>
> See ignored test associated with this ticket.
> {noformat}
> [vm2] [warn 2020/07/07 17:12:48.216 EDT <ResourceManagerRecoveryThread 1>
> tid=0x46] 15 seconds have elapsed while waiting for replies:
> <DeposePrimaryBucketMessage$DeposePrimaryBucketResponse 1171 waiting for 1
> replies from [192.168.0.104(server-1:27717)<v1>:41001]> on
> 192.168.0.104(server-2:27730)<v9>:41002 whose current membership list is:
> [[192.168.0.104(locator-0:27716:locator)<ec><v0>:41000,
> 192.168.0.104(server-1:27717)<v1>:41001,
> 192.168.0.104(server-2:27730)<v9>:41002,
> 192.168.0.104(server-3:27731)<v11>:41003]]
> [vm2] [warn 2020/07/07 17:12:48.216 EDT <Function Execution Processor3>
> tid=0x5c] 15 seconds have elapsed while waiting for replies:
> <PRFunctionStreamingResultCollector 1170 waiting for 1 replies from
> [192.168.0.104(server-1:27717)<v1>:41001]> on
> 192.168.0.104(server-2:27730)<v9>:41002 whose current membership list is:
> [[192.168.0.104(locator-0:27716:locator)<ec><v0>:41000,
> 192.168.0.104(server-1:27717)<v1>:41001,
> 192.168.0.104(server-2:27730)<v9>:41002,
> 192.168.0.104(server-3:27731)<v11>:41003]]
> {noformat}
> The Redis RENAME command could hang during a rebalance, if the old key was
> stored in a bucket on one server, and the new key was in a different bucket
> on a separate server. Rename would read-lock the buckets, but rebalance would
> wait for a write lock, and the Rename region put/destroy would then wait on
> the write lock.
> Now Rename passes a callback argument that indicates it has already locked
> the primary, and will not attempt to lock the primary again when doing the
> put/destroy.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
