CP-28406: improve logging and error checking in clustering functions #3614
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
minishrink
approved these changes
May 31, 2018
| Xapi_clustering.Daemon.disable ~__context | ||
| | Result.Error error -> | ||
| D.warn "Error occurred during Cluster.destroy"; | ||
| handle_error error |
There was a problem hiding this comment.
This is good, we were unnecessarily duplicating code.
| Xapi_clustering.Daemon.disable ~__context | ||
| | Result.Error error -> | ||
| warn "Error occurred during Cluster_host.force_destroy"; | ||
| handle_error error |
ocaml/xapi/xapi_clustering.ml
Outdated
| ) srs | ||
| then raise Api_errors.(Server_error (cluster_stack_in_use, [ cluster_stack ])) | ||
|
|
||
| let daemon_enabled = ref false |
There was a problem hiding this comment.
Do we want a helper to query this?
There was a problem hiding this comment.
Actually ignore that, the code is good to merge
|
Just realized that we do need to prevent Clusters without cluster hosts, otherwise pool autojoin won't work, and in fact a Cluster object without cluster hosts is unusable. |
|
We should merge #3607 first and then this one needs to be rebased. |
gaborigloi
approved these changes
Jun 5, 2018
Locking_helpers provides a nice list of active locks as part of `xe host-get-thread-diagnostics`, which helps debugging. Signed-off-by: Edwin Török <edvin.torok@citrix.com>
This is quite useful for tracing what Xapi_cluster_host methods are called, and together with the previous commit for tracking deadlocks. Signed-off-by: Edwin Török <edvin.torok@citrix.com>
…ustering daemon is down Cluster.destroy did not work if you destroyed the last cluster host (like some of the unit tests actually did). Cluster_host.destroy on the last node is special: there is no cluster after you leave (leave would fail in reality), so just destroy the cluster. Refactor all 3 destroy operations into one, choose automatically based on number of hosts. Could've introduced a new API error to forbid destroying the last cluster host, but it is better if XAPI is able to automatically do the right thing than to tell the user it should call some other API instead. Also with Cluster_host.force_destroy you could have already ended up in a situation where you have no cluster hosts and want to destroy the cluster, which would hang indefinitely because the daemon was stopped. We always try to enable the daemon on startup, so keep track on whether we think it should be running, and if we know we stopped it then just raise an error when trying to do an RPC. This is useful in debugging situations where we try to send RPCs too early too (e.g. before we started the daemon). Signed-off-by: Edwin Török <edvin.torok@citrix.com>
|
Rebased on latest branch. |
edwintorok
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was useful in debugging problems with @minishrink PR.