You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by koushik-das <gi...@git.apache.org> on 2015/08/04 09:31:49 UTC

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

GitHub user koushik-das opened a pull request:

    https://github.com/apache/cloudstack/pull/656

    CLOUDSTACK-8704: Schedule restart of router VMs ahead of user VMs as …

    …part of HA
    
    VRs are scheduled for HA ahead of user VMs.
    
    Refer to the bug for more details.

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

    $ git pull https://github.com/koushik-das/cloudstack CLOUDSTACK-8704

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

    https://github.com/apache/cloudstack/pull/656.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 #656
    
----
commit 032a3353f7dc05c859954b4eb58398fb166de554
Author: Koushik Das <ko...@apache.org>
Date:   2015-08-04T07:29:09Z

    CLOUDSTACK-8704: Schedule restart of router VMs ahead of user VMs as part of HA
    VRs are scheduled for HA ahead of user VMs

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128686334
  
    @kishankavala Made the change to use VM type instead of name to distinguish between user and system VMs


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128491021
  
    @koushik-das Thanks for the fix! I was testing this on KVM tonight but run into an unrelated issue. Will try to verify your fix tomorrow so we can merge it soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129335984
  
    @remibergsma I have fixed the commits, please check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129334661
  
    @koushik-das Check. If you fix the commits, I'll LGTM and merge. Let me know if you need help.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128748550
  
    Thanks @miguelaferreira :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/656#discussion_r36168074
  
    --- Diff: server/test/com/cloud/ha/HighAvailabilityManagerImplTest.java ---
    @@ -113,12 +110,18 @@
         public void setup() throws IllegalArgumentException,
                 IllegalAccessException, NoSuchFieldException, SecurityException {
             highAvailabilityManager = new HighAvailabilityManagerImpl();
    -        for (Field injectField : HighAvailabilityManagerImpl.class
    -                .getDeclaredFields()) {
    +        for (Field injectField : HighAvailabilityManagerImpl.class.getDeclaredFields()) {
                 if (injectField.isAnnotationPresent(Inject.class)) {
                     injectField.setAccessible(true);
    -                injectField.set(highAvailabilityManager, this.getClass()
    -                        .getDeclaredField(injectField.getName()).get(this));
    +                injectField.set(highAvailabilityManager, this.getClass().getDeclaredField(injectField.getName()).get(this));
    +            } else if (injectField.getName().equals("_workers")) {
    --- End diff --
    
    I suppose this is needed for the extra tests. Can you explain?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/656#discussion_r36382437
  
    --- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
    @@ -242,16 +242,19 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate)
     
             // send an email alert that the host is down
             StringBuilder sb = null;
    +        List<VMInstanceVO> reorderedVMList = new ArrayList<VMInstanceVO>();
             if ((vms != null) && !vms.isEmpty()) {
                 sb = new StringBuilder();
    -            sb.append("  Starting HA on the following VMs: ");
    +            sb.append("  Starting HA on the following VMs:");
                 // collect list of vm names for the alert email
    -            VMInstanceVO vm = vms.get(0);
    -            if (vm.isHaEnabled()) {
    -                sb.append(" " + vm);
    -            }
    -            for (int i = 1; i < vms.size(); i++) {
    -                vm = vms.get(i);
    +            for (int i = 0; i < vms.size(); i++) {
    +                VMInstanceVO vm = vms.get(i);
    +                String name = vm.getInstanceName();
    +                if (name.startsWith("i-")) {
    --- End diff --
    
    @kishankavala Although the issue was seen only with VR, the fix is to schedule restart of system VMs ahead of user VMs.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/656#discussion_r36168376
  
    --- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
    @@ -261,25 +264,21 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate)
             // send an email alert that the host is down, include VMs
             HostPodVO podVO = _podDao.findById(host.getPodId());
             String hostDesc = "name: " + host.getName() + " (id:" + host.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName();
    +        _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc,
    +                "Host [" + hostDesc + "] is down." + ((sb != null) ? sb.toString() : ""));
     
    -        _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc, "Host [" + hostDesc +
    -            "] is down." +
    -            ((sb != null) ? sb.toString() : ""));
    -
    -        if (vms != null) {
    -            for (VMInstanceVO vm : vms) {
    -                if (s_logger.isDebugEnabled()) {
    -                    s_logger.debug("Notifying HA Mgr of to restart vm " + vm.getId() + "-" + vm.getInstanceName());
    -                }
    -                vm = _instanceDao.findByUuid(vm.getUuid());
    -                Long hostId = vm.getHostId();
    -                if (hostId != null && !hostId.equals(host.getId())) {
    -                    s_logger.debug("VM " + vm.getInstanceName() + " is not on down host " + host.getId() + " it is on other host "
    -                            + hostId + " VM HA is done");
    -                    continue;
    -                }
    -                scheduleRestart(vm, investigate);
    +        for (VMInstanceVO vm : reorderedVMList) {
    +            if (s_logger.isDebugEnabled()) {
    +                s_logger.debug("Notifying HA Mgr of to restart vm " + vm.getId() + "-" + vm.getInstanceName());
    +            }
    +            vm = _instanceDao.findByUuid(vm.getUuid());
    +            Long hostId = vm.getHostId();
    +            if (hostId != null && !hostId.equals(host.getId())) {
    --- End diff --
    
    @DaanHoogland The host check you are referring to existed even before this PR. One reason for the check is to ensure that the VM has not moved to another host since the time it was fetched using listByHostId(). If it has moved to another host (and vmsync has updated the host id) then no need to perform restart/HA.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128828795
  
    Hi @koushik-das I was able to test your PR and it seems to do what you said: start the system VMs before the other instances. However, it doesn't solve the problem completely. In my case, I had two routers and a bunch of VMs. The routers started first, but when the first instance was started the router was not yet ready. CloudStack then destroyed the router, and on the next HA run about 5-10 minutes later everything was started properly. If you want my full logs, let me know and I'll put them somewhere.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by kishankavala <gi...@git.apache.org>.
Github user kishankavala commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/656#discussion_r36381969
  
    --- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
    @@ -242,16 +242,19 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate)
     
             // send an email alert that the host is down
             StringBuilder sb = null;
    +        List<VMInstanceVO> reorderedVMList = new ArrayList<VMInstanceVO>();
             if ((vms != null) && !vms.isEmpty()) {
                 sb = new StringBuilder();
    -            sb.append("  Starting HA on the following VMs: ");
    +            sb.append("  Starting HA on the following VMs:");
                 // collect list of vm names for the alert email
    -            VMInstanceVO vm = vms.get(0);
    -            if (vm.isHaEnabled()) {
    -                sb.append(" " + vm);
    -            }
    -            for (int i = 1; i < vms.size(); i++) {
    -                vm = vms.get(i);
    +            for (int i = 0; i < vms.size(); i++) {
    +                VMInstanceVO vm = vms.get(i);
    +                String name = vm.getInstanceName();
    +                if (name.startsWith("i-")) {
    --- End diff --
    
    @koushik-das To identify VRs in this list, vm.getType().equals(VirtualMachine.Type.DomainRouter) would be better, instead of relying on the Vm name. Also, in the mentioned code SSVM and CPVM get higher priority than UserVms.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129368352
  
    @koushik-das Thanks, glad the merge commit is gone now. Do you want to keep two commits? I'd say squash them together. From the subject line I cannot tell the difference. Let me know and I'll proceed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129365283
  
    Travis failure is due to timeout.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-127529005
  
    code LGTM.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128727494
  
    @koushik-das you can solve it like this:
    - start with a new branch from your first commit
    - then cherry-pick the second one in
    - consider if you want to squash them together
    - force push this to your fork (koushik-das) on branch (CLOUDSTACK-8704)
    The PR should then have two commits. Let me know if you need any help!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128748395
  
    Hi @koushik-das 
    
    Sorry to step in like this, but I talked to @remibergsma and I think you can fix your branch by doing this:
    ```
    git checkout 032a335
    git cherry-pick cabc9ea
    git reset --hard
    ```
    
    If you do this, please double-check the result (it should be your two commits together at the top), and then do: 
    ```
    git push -f origin HEAD:CLOUDSTACK-8704
    ```
    
    To update this PR.
    
    I hope this helps.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129300344
  
    @remibergsma Thanks for testing the PR. This is a best-effort fix. The change just schedules the restart of the system VMs ahead of the user VMs. Now if there are fewer VMs for restart, there is a higher probability of all restart jobs getting executed at the same time by the HA-worker thread pool. In this case the fix won't be effective. If there are more VMs then the effectiveness of the fix increases.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/656#discussion_r36167770
  
    --- Diff: server/src/com/cloud/ha/HighAvailabilityManagerImpl.java ---
    @@ -261,25 +264,21 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate)
             // send an email alert that the host is down, include VMs
             HostPodVO podVO = _podDao.findById(host.getPodId());
             String hostDesc = "name: " + host.getName() + " (id:" + host.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName();
    +        _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc,
    +                "Host [" + hostDesc + "] is down." + ((sb != null) ? sb.toString() : ""));
     
    -        _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc, "Host [" + hostDesc +
    -            "] is down." +
    -            ((sb != null) ? sb.toString() : ""));
    -
    -        if (vms != null) {
    -            for (VMInstanceVO vm : vms) {
    -                if (s_logger.isDebugEnabled()) {
    -                    s_logger.debug("Notifying HA Mgr of to restart vm " + vm.getId() + "-" + vm.getInstanceName());
    -                }
    -                vm = _instanceDao.findByUuid(vm.getUuid());
    -                Long hostId = vm.getHostId();
    -                if (hostId != null && !hostId.equals(host.getId())) {
    -                    s_logger.debug("VM " + vm.getInstanceName() + " is not on down host " + host.getId() + " it is on other host "
    -                            + hostId + " VM HA is done");
    -                    continue;
    -                }
    -                scheduleRestart(vm, investigate);
    +        for (VMInstanceVO vm : reorderedVMList) {
    +            if (s_logger.isDebugEnabled()) {
    +                s_logger.debug("Notifying HA Mgr of to restart vm " + vm.getId() + "-" + vm.getInstanceName());
    +            }
    +            vm = _instanceDao.findByUuid(vm.getUuid());
    +            Long hostId = vm.getHostId();
    +            if (hostId != null && !hostId.equals(host.getId())) {
    --- End diff --
    
    no biggy for this PR, but why is this chack needed? The vms are retrieved by the host id of the host being down in the first place (see line 240).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129396359
  
    @koushik-das Thank you!
    LGTM, merging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/656


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129371706
  
    @remibergsma Yes the commits can be squashed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128719973
  
    @koushik-das I' testing this tonight and will merge if it's OK. Before I proceed, could you please squash the 3 commits into 1. I cannot merge it like this. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-128745820
  
    @remibergsma Will the following work?
    - do git pull on master
    - then do a git pull --rebase, this will make the commits appear on top
    - then do the squash
    - push them to master after editing the message


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129405117
  
    @remibergsma ahum,... LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129407180
  
    @DaanHoogland You already gave your LGTM? See here: https://github.com/apache/cloudstack/pull/656#issuecomment-127529005



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/656#discussion_r36170828
  
    --- Diff: server/test/com/cloud/ha/HighAvailabilityManagerImplTest.java ---
    @@ -113,12 +110,18 @@
         public void setup() throws IllegalArgumentException,
                 IllegalAccessException, NoSuchFieldException, SecurityException {
             highAvailabilityManager = new HighAvailabilityManagerImpl();
    -        for (Field injectField : HighAvailabilityManagerImpl.class
    -                .getDeclaredFields()) {
    +        for (Field injectField : HighAvailabilityManagerImpl.class.getDeclaredFields()) {
                 if (injectField.isAnnotationPresent(Inject.class)) {
                     injectField.setAccessible(true);
    -                injectField.set(highAvailabilityManager, this.getClass()
    -                        .getDeclaredField(injectField.getName()).get(this));
    +                injectField.set(highAvailabilityManager, this.getClass().getDeclaredField(injectField.getName()).get(this));
    +            } else if (injectField.getName().equals("_workers")) {
    --- End diff --
    
    Yes. "_workers" is the set of worker threads that perform HA related tasks in HighAvaiabilityManagerImpl class. As part of the test wakeupWorkers() methods get invoked which needs _workers to be initialized.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8704: Schedule restart of rout...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/656#issuecomment-129374324
  
    @koushik-das Check, please squash and then force push again. After that I'll merge it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---