[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

jbertram-2
GitHub user gaohoward opened a pull request:

    https://github.com/apache/activemq-artemis/pull/2041

    ARTEMIS-1825 Live-backup topology not correctly displayed on console

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/gaohoward/activemq-artemis h1825

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/2041.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2041
   
----
commit 82241555d48ece29a76b6b48da8921225e747d87
Author: Howard Gao <howard.gao@...>
Date:   2018-04-24T04:08:33Z

    ARTEMIS-1825 Live-backup topology not correctly displayed on console

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

jbertram-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2041#discussion_r184175183
 
    --- Diff: artemis-hawtio/artemis-plugin/src/main/webapp/plugin/js/brokerDiagram.js ---
    @@ -455,16 +457,27 @@ var ARTEMIS = (function(ARTEMIS) {
                                   addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.live + "\"", "network");
     
                                   var backup = remoteBroker.backup;
    +                              ARTEMIS.log.info("isbackup? " + backup);
                                   if (backup) {
                                      getOrAddBroker(false, "\"" + backup + "\"", remoteBroker.nodeID, "remote", null, properties);
                                      addLinkIds("broker:" + "\"" + remoteBroker.live + "\"", "broker:" + "\"" + backup + "\"", "network");
                                   }
                                }
                                else {
    -                              var backup = remoteBroker.backup;
    -                              if (backup) {
    -                                 getOrAddBroker(false, "\"" + remoteBroker.backup + "\"", remoteBroker.nodeID, "remote", null, properties);
    -                                 addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.backup + "\"", "network");
    +                              var newBackReq = ARTEMISService.artemisConsole.isBackup(jolokia, mBean);
    +                              var newBackup = newBackReq.value;
    +                              if (!newBackup) {
    +                                 ARTEMIS.log.info("yes I'm master right now");
    --- End diff --
   
    this is a debug message.. you probably didn't mean to have it here.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

jbertram-2
In reply to this post by jbertram-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2041#discussion_r184175230
 
    --- Diff: artemis-hawtio/artemis-plugin/src/main/webapp/plugin/js/brokerDiagram.js ---
    @@ -455,16 +457,27 @@ var ARTEMIS = (function(ARTEMIS) {
                                   addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.live + "\"", "network");
     
                                   var backup = remoteBroker.backup;
    +                              ARTEMIS.log.info("isbackup? " + backup);
                                   if (backup) {
                                      getOrAddBroker(false, "\"" + backup + "\"", remoteBroker.nodeID, "remote", null, properties);
                                      addLinkIds("broker:" + "\"" + remoteBroker.live + "\"", "broker:" + "\"" + backup + "\"", "network");
                                   }
                                }
                                else {
    -                              var backup = remoteBroker.backup;
    -                              if (backup) {
    -                                 getOrAddBroker(false, "\"" + remoteBroker.backup + "\"", remoteBroker.nodeID, "remote", null, properties);
    -                                 addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.backup + "\"", "network");
    +                              var newBackReq = ARTEMISService.artemisConsole.isBackup(jolokia, mBean);
    +                              var newBackup = newBackReq.value;
    +                              if (!newBackup) {
    +                                 ARTEMIS.log.info("yes I'm master right now");
    +                                 if (remoteBroker.backup) {
    +                                    getOrAddBroker(false, "\"" + remoteBroker.backup + "\"", remoteBroker.nodeID, "remote", null, properties);
    +                                    addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.backup + "\"", "network");
    +                                 }
    +                              }
    +                              else {
    +                                 ARTEMIS.log.info("ok I'm backup!");
    --- End diff --
   
    debug message


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

jbertram-2
In reply to this post by jbertram-2
Github user gaohoward commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2041#discussion_r184259404
 
    --- Diff: artemis-hawtio/artemis-plugin/src/main/webapp/plugin/js/brokerDiagram.js ---
    @@ -455,16 +457,27 @@ var ARTEMIS = (function(ARTEMIS) {
                                   addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.live + "\"", "network");
     
                                   var backup = remoteBroker.backup;
    +                              ARTEMIS.log.info("isbackup? " + backup);
                                   if (backup) {
                                      getOrAddBroker(false, "\"" + backup + "\"", remoteBroker.nodeID, "remote", null, properties);
                                      addLinkIds("broker:" + "\"" + remoteBroker.live + "\"", "broker:" + "\"" + backup + "\"", "network");
                                   }
                                }
                                else {
    -                              var backup = remoteBroker.backup;
    -                              if (backup) {
    -                                 getOrAddBroker(false, "\"" + remoteBroker.backup + "\"", remoteBroker.nodeID, "remote", null, properties);
    -                                 addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.backup + "\"", "network");
    +                              var newBackReq = ARTEMISService.artemisConsole.isBackup(jolokia, mBean);
    +                              var newBackup = newBackReq.value;
    +                              if (!newBackup) {
    +                                 ARTEMIS.log.info("yes I'm master right now");
    +                                 if (remoteBroker.backup) {
    +                                    getOrAddBroker(false, "\"" + remoteBroker.backup + "\"", remoteBroker.nodeID, "remote", null, properties);
    +                                    addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.backup + "\"", "network");
    +                                 }
    +                              }
    +                              else {
    +                                 ARTEMIS.log.info("ok I'm backup!");
    --- End diff --
   
    Yes should be debug. I'll change that. Thx.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user gaohoward commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    @clebertsuconic done.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

jbertram-2
In reply to this post by jbertram-2
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2041#discussion_r184398314
 
    --- Diff: artemis-hawtio/artemis-plugin/src/main/webapp/plugin/js/brokerDiagram.js ---
    @@ -461,10 +463,20 @@ var ARTEMIS = (function(ARTEMIS) {
                                   }
                                }
                                else {
    -                              var backup = remoteBroker.backup;
    -                              if (backup) {
    -                                 getOrAddBroker(false, "\"" + remoteBroker.backup + "\"", remoteBroker.nodeID, "remote", null, properties);
    -                                 addLinkIds("broker:" + broker.brokerId, "broker:" + "\"" + remoteBroker.backup + "\"", "network");
    +                              var newBackReq = ARTEMISService.artemisConsole.isBackup(jolokia, mBean);
    +                              var newBackup = newBackReq.value;
    +                              if (!newBackup) {
    +                                 ARTEMIS.log.debug("yes I'm master right now");
    --- End diff --
   
    You still have unintended debug messages here


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user gaohoward commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    @clebertsuconic removed the debug logs.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    @mtaylor / @andytaylor  / @michaelandrepearce  I didn't work on this module a lot... can you guys check on this?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    @gaohoward sorry taken a while to look at this as have been burning hours on other issue. From what i can tell from the JS this looks ok, i assume form reading the js, the idea here is more clearly show who is live and who is a slave, calling a method to deduce this, rather than the hardcoded master=true, is this roughly a correct understanding? If so +1 from me.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user gaohoward commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    @michaelandrepearce  Yes that's the idea.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    Great ill merge this shortly then. Thanks for the contribution, and sorry again for taking soo long to review it


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2041: ARTEMIS-1825 Live-backup topology not correctl...

jbertram-2
In reply to this post by jbertram-2
Github user gaohoward commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2041
 
    np at all.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2041: ARTEMIS-1825 Live-backup topology not c...

jbertram-2
In reply to this post by jbertram-2
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/2041


---