Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Allocation Delete failures now save metrics if possible. #34

Merged
merged 1 commit into from
May 10, 2018

Conversation

mew2057
Copy link
Contributor

@mew2057 mew2057 commented May 9, 2018

Candidate fix for Issue #31:

Implemented a new mechanism for handling softer failures in multicast code. Allocation Delete failures will now maintain the metrics after failing.

Added a new field _ErrorMsg to the multicast properties template class to facilitate this mechanism.

@@ -117,15 +117,14 @@ bool AllocationAgentUpdateState::HandleNetworkMessage(
}

// If the initialization or reversion was successful reply to the master daemon.
if ( success )
if ( success || response->error_code != CSMI_SUCCESS )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if success or not success? Only line i'm confused about. otherwise nothing jumps out as crazy. but I don't know enough about this part of the code to offer any real comment of substance.

explain this to me then I will be ready to approve.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section effectively says that if the RevertNode or InitNode returned true (meaning the function completed in a good execution) respond. Additionally, if the response has an error code, meaning the RevertNode executed, but it had problems, we want to respond with the data enriched path.

By having both options in the if statement we capture cases where there's no data enrichment, but a failure was found in one of the functions or before the functions got called (response->error_code defaults to CSMI_SUCCESS). Additionally, we don't skip data enriched payloads just because the function returned an error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ight

Copy link
Contributor

@fpizzano fpizzano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build Pull request and tested.
Now I see the correct information returning from csm_allocation_query_details and at CSMDB.

Copy link
Contributor

@pdlun92 pdlun92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passes regression at this point. If there are any additional commits I can re-run. Approving

@NickyDaB
Copy link
Contributor

Looks like everyone approves. So, I guess you are cleared for pull.

@mew2057 mew2057 merged commit f2f73f3 into IBM:master May 10, 2018
@mew2057 mew2057 deleted the allocation-delete-remote-enhancement branch May 10, 2018 14:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants