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
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
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
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
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 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
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
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
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
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
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
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
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
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 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
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
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
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
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
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
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
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
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,
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
24 matches
Mail list logo