You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/02/25 20:17:54 UTC

[GitHub] [geode-native] mmartell opened a new pull request #936: GEODE-10085: Don't start JmxManager if already running

mmartell opened a new pull request #936:
URL: https://github.com/apache/geode-native/pull/936


   This adds support for multiple locators in the new .NET test framework.


-- 
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



[GitHub] [geode-native] mmartell commented on pull request #936: GEODE-10085: Don't start JmxManager if already running

Posted by GitBox <gi...@apache.org>.
mmartell commented on pull request #936:
URL: https://github.com/apache/geode-native/pull/936#issuecomment-1054691256


   Converting to Draft until fix also implemented in new cppcache test framework.


-- 
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



[GitHub] [geode-native] mmartell commented on a change in pull request #936: GEODE-10085: Don't start JmxManager if already running

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #936:
URL: https://github.com/apache/geode-native/pull/936#discussion_r817006325



##########
File path: clicache/integration-test2/Cluster.cs
##########
@@ -70,8 +70,9 @@ private bool StartLocators()
 
             for (var i = 0; i < locatorCount_; i++)
             {
+                var startJmxManager = (i == 0);

Review comment:
       i) You can't start the JmxManager in gfsh independent of a locator.
   iii) Good catch! StartServers() has same issue. Fixed that one also.




-- 
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



[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #936: GEODE-10085: Don't start JmxManager if already running

Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #936:
URL: https://github.com/apache/geode-native/pull/936#discussion_r816872113



##########
File path: clicache/integration-test2/Cluster.cs
##########
@@ -70,8 +70,9 @@ private bool StartLocators()
 
             for (var i = 0; i < locatorCount_; i++)
             {
+                var startJmxManager = (i == 0);

Review comment:
       Okay, a few problems here:
   i) I'd really like to do this without introducing a boolean parameter, though I understand it may be unavoidable.
   ii) We don't need a new startJmxManager here, just pass i == 0 as the last parameter to the locator ctor
   iii) This was here before you got to it, but this loop is still broken for multiple locators, because the success variable is broken.  If you start two, and the first one fails but the second one works, you've buried an error.  Please break out of the for loop if success is false.




-- 
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



[GitHub] [geode-native] mmartell commented on a change in pull request #936: GEODE-10085: Don't start JmxManager if already running

Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #936:
URL: https://github.com/apache/geode-native/pull/936#discussion_r817006325



##########
File path: clicache/integration-test2/Cluster.cs
##########
@@ -70,8 +70,9 @@ private bool StartLocators()
 
             for (var i = 0; i < locatorCount_; i++)
             {
+                var startJmxManager = (i == 0);

Review comment:
       i) You can't start the JmxManager in gfsh independent of a locator.
   iii) Good catch!




-- 
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



[GitHub] [geode-native] mmartell merged pull request #936: GEODE-10085: Don't start JmxManager if already running

Posted by GitBox <gi...@apache.org>.
mmartell merged pull request #936:
URL: https://github.com/apache/geode-native/pull/936


   


-- 
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