Github user abhinandanprateek commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98148840
Another thing to note is that while changing some of this legacy
understanding, it is not just the code that needs improvement/change, it is all
the existin
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98143582
@dahn you're right, I looked at the code and made some semantic changes
around debugging/logging and return value type. I guess there are some parts in
the source co
I finally got around a further code analysis and must conclude that
returning ALERT is no problem.
Status testIpAddress(Long hostId, String testHostIp), is called twice and
in both cases null is checked for avoiding NPE and next only the cases UP
and DOWN are handled. So the behaviour on a null re
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98127447
Please don't revert. It is on the 4.4 branch as well. It passes all tests
and we have yet to create a test that show this hypothetical regression. If we
have I'l
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98126407
Guys - do we have any consensus on this issue? I'm planning to cut a 4.5.1
RC on Monday, if we are not very clear I would like to revert this on 4.5 to
avoid any reg
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98120238
@remibergsma Yes XS (6.5) pool HA was enabled.
@DaanHoogland Agree on the new state UNKNOWN
---
If your project is set up for it, you can reply to this e
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98104904
Koushik, I'd rather have the state machine never return null and create a
state UNKNOWN. The alert.wait parameter can check for this before setting
the sta
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98104693
@koushik-das Since CloudStack 4.4 XenHA is the one to elect the new pool
master. Before, CloudStack would force an election. Personally I do not like
this new beh
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98100569
One possible solution I can think of the original problem for this PR is to
wait for sometime (say based on the config parameter alert.wait) and if even
after tha
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98100363
I looked at the code and saw that if investigate() is not able to determine
the state of host then by default Alert is returned, earlier it was null. This
may not
Github user abhinandanprateek commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98093700
Daan,
Yes, I agree. We can define a proper state and introduce it. I guess
there is some tribal knowledge involved here that Koushik and myself were
Github user abhinandanprateek commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98091401
I agree with Koushik here. The purpose of an HA investigator is to either
say that host is UP or Down or it does not know for sure. A host that has not
ret
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98091566
Abhinandan, I think that is a design flow and we should revisit. I doubt
that it is relevant to this fix however. The stjatemachine was resetting
the state
Github user abhinandanprateek commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-98087158
The way HA is implemented, ânullâ is a valid return value from an
investigator. The valid values are Up, Down or null.
https://cwiki.apache.org/conf
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97542905
@mlsorensen Agree, no state of VMs should change since we cannot tell
because the hypervisors are unreachable. This does not change the state of the
VMs, only the
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97512774
Also about the scenario that is being addressed - all hosts in a XS cluster
are shutdown by pulling the plug. Instead of immediately putting the hosts in
Alert st
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97503915
@remibergsma My comments are based on the latest master code. Based on my
reading of the code, the changes you made shouldn't have any impact on the
outcome of th
Github user mlsorensen commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97475041
I guess for KVM at least, the behavior should be similar to
stopping/starting the Agent. It makes the hypervisor go into disconnected, but
leaves the VM states as-
Github user mlsorensen commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97474185
I haven't researched this fix extensively. However, I just want to make
sure that the alert state is not accompanied by any undesirable behavior.
Moving the hyperv
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97471962
@koushik-das To reproduce the issue:
- create a cluster of 1 or more hypervisors. We observed this issue using
xenserver.
- spin up some vms on this cluste
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97454863
@DaanHoogland If you look at the code from where testIpAddress() is called
(for e.g. UserVmDomRInvestigator.isAgentAlive()), it only deals with return of
Up, Down
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97453854
@koushik-das what is your point? I think I miss context for your remark.
---
If your project is set up for it, you can reply to this email and have your
reply ap
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97452036
Reading the code, if testIpAddress() returns anything other than Up, Down
or null then returned status is ignored.
---
If your project is set up for it, you can
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/211
---
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
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/211#issuecomment-97416661
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
e
GitHub user remibergsma opened a pull request:
https://github.com/apache/cloudstack/pull/211
return a state instead of null in AbstractInvestigatorImpl
When a full cluster is down or unreachable,
CloudStack currently reports everything the
same as the last known state, which
26 matches
Mail list logo