[jira] [Commented] (GEODE-10287) DistributedRegion.distributedRegionCleanup logic looks wrong
[ https://issues.apache.org/jira/browse/GEODE-10287?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17539655#comment-17539655 ] Jinmei Liao commented on GEODE-10287: - Oh, yeah, you are right. The test specifically blocks the operation and cause the `waitForCurrentOperations` to wait forever, that's why the test hang. If we close the distadvisor before the wait, then the wait is a no-op. then the test succeeds. > DistributedRegion.distributedRegionCleanup logic looks wrong > > > Key: GEODE-10287 > URL: https://issues.apache.org/jira/browse/GEODE-10287 > Project: Geode > Issue Type: Bug > Components: core >Reporter: Darrel Schneider >Assignee: Jinmei Liao >Priority: Major > > DistributedRegion.distributedRegionCleanup does this: distAdvisor.close(). > Then a few lines later it calls "waitForCurrentOperations()". But > waitForCurrentOperations uses the closed distAdvisor. Maybe it is okay to > uses a closed distAdvisor but it seems better to call wait first and then > close distAdvisor. -- This message was sent by Atlassian Jira (v8.20.7#820007)
[jira] [Commented] (GEODE-10287) DistributedRegion.distributedRegionCleanup logic looks wrong
[ https://issues.apache.org/jira/browse/GEODE-10287?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17539644#comment-17539644 ] Darrel Schneider commented on GEODE-10287: -- DistributionAdvisor.close calls operationMonitor.close which sets the "closed" field on it to true. So when we call forceNewMembershipVersion it is a noop. waitForCurrentOperations is also a noop because operationsAreInProgress will now always return false because operationMonitor.close set previousVersionOpCount to 0. > DistributedRegion.distributedRegionCleanup logic looks wrong > > > Key: GEODE-10287 > URL: https://issues.apache.org/jira/browse/GEODE-10287 > Project: Geode > Issue Type: Bug > Components: core >Reporter: Darrel Schneider >Assignee: Jinmei Liao >Priority: Major > > DistributedRegion.distributedRegionCleanup does this: distAdvisor.close(). > Then a few lines later it calls "waitForCurrentOperations()". But > waitForCurrentOperations uses the closed distAdvisor. Maybe it is okay to > uses a closed distAdvisor but it seems better to call wait first and then > close distAdvisor. -- This message was sent by Atlassian Jira (v8.20.7#820007)
[jira] [Commented] (GEODE-10287) DistributedRegion.distributedRegionCleanup logic looks wrong
[ https://issues.apache.org/jira/browse/GEODE-10287?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17539627#comment-17539627 ] Jinmei Liao commented on GEODE-10287: - For a closed distAdvisor, the call to the `forceNewMembershipVersion` will do something since it's calling ` operationMonitor.forceNewMembershipVersion();`, the operationMonitor is not closed, so it will increment the ` previousVersionOpCount`, this prevents the `waitForCurrentOperations` to wait if the distAdvisor is not closed. > DistributedRegion.distributedRegionCleanup logic looks wrong > > > Key: GEODE-10287 > URL: https://issues.apache.org/jira/browse/GEODE-10287 > Project: Geode > Issue Type: Bug > Components: core >Reporter: Darrel Schneider >Assignee: Jinmei Liao >Priority: Major > > DistributedRegion.distributedRegionCleanup does this: distAdvisor.close(). > Then a few lines later it calls "waitForCurrentOperations()". But > waitForCurrentOperations uses the closed distAdvisor. Maybe it is okay to > uses a closed distAdvisor but it seems better to call wait first and then > close distAdvisor. -- This message was sent by Atlassian Jira (v8.20.7#820007)
[jira] [Commented] (GEODE-10287) DistributedRegion.distributedRegionCleanup logic looks wrong
[ https://issues.apache.org/jira/browse/GEODE-10287?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17537840#comment-17537840 ] Darrel Schneider commented on GEODE-10287: -- I'm okay with not fixing this for 1.15. But something is wrong. At most all waitForCurrentOperations will do is call forceNewMembershipVersion and waitForCurrentOperations on the closed advisor. But both of these methods will do nothing if the advisor is closed because the underlying operationMonitor was also closed. So if the current code is correct then we should not even bother calling waitForCurrentOperations in this method since it is ALWAYS a noop. But I don't know if we actually need to call a functional waitForCurrentOperations here. If we do then the hangs you found would need to be addressed. > DistributedRegion.distributedRegionCleanup logic looks wrong > > > Key: GEODE-10287 > URL: https://issues.apache.org/jira/browse/GEODE-10287 > Project: Geode > Issue Type: Bug > Components: core >Reporter: Darrel Schneider >Assignee: Jinmei Liao >Priority: Major > Labels: needsTriage > > DistributedRegion.distributedRegionCleanup does this: distAdvisor.close(). > Then a few lines later it calls "waitForCurrentOperations()". But > waitForCurrentOperations uses the closed distAdvisor. Maybe it is okay to > uses a closed distAdvisor but it seems better to call wait first and then > close distAdvisor. -- This message was sent by Atlassian Jira (v8.20.7#820007)
[jira] [Commented] (GEODE-10287) DistributedRegion.distributedRegionCleanup logic looks wrong
[ https://issues.apache.org/jira/browse/GEODE-10287?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17536337#comment-17536337 ] Jinmei Liao commented on GEODE-10287: - If I close the distAdvisor after "waitForCurrentOperations()", the ConnectionCloseSSLTLSDUnitTest.connectionWithHungReaderIsCloseableAndUnhangsReader() will hang, that test will block the current operation, and make sure cache is still closable. If we waitForCurrentOperations before we close the distAdvisor, it will wait forever. > DistributedRegion.distributedRegionCleanup logic looks wrong > > > Key: GEODE-10287 > URL: https://issues.apache.org/jira/browse/GEODE-10287 > Project: Geode > Issue Type: Bug > Components: core >Reporter: Darrel Schneider >Assignee: Jinmei Liao >Priority: Major > Labels: needsTriage > > DistributedRegion.distributedRegionCleanup does this: distAdvisor.close(). > Then a few lines later it calls "waitForCurrentOperations()". But > waitForCurrentOperations uses the closed distAdvisor. Maybe it is okay to > uses a closed distAdvisor but it seems better to call wait first and then > close distAdvisor. -- This message was sent by Atlassian Jira (v8.20.7#820007)
