HDDS-1146. Adding container related metrics in SCM. #1541
Conversation
| * @param isFullReport | ||
| * @param success | ||
| */ | ||
| void notifyContainerReportProcessing(boolean isFullReport, boolean success); |
There was a problem hiding this comment.
I am not sure, But I think we have a Enum that indicates if the report is full or partial. If there is already one, then we might want to use it. Otherwise, I am okay with the boolean.
There was a problem hiding this comment.
From proto, there is no enum. But i see we have TypedEvent
TypedEvent
INCREMENTAL_CONTAINER_REPORT = new TypedEvent<>(
IncrementalContainerReportFromDatanode.class,
"Incremental_Container_Report")
TypedEvent CONTAINER_REPORT =
new TypedEvent<>(ContainerReportFromDatanode.class, "Container_Report");
Do you mean we want to use them instead of boolean?
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@bharatviswa504 If you don't mind, can you tell me where a metric like numContainerReportsSuccess would be useful? Failures I guess may be useful. |
| LOG.error("Exception while processing ICR for container {}", | ||
| replicaProto.getContainerID()); | ||
| } | ||
| } | ||
|
|
||
| if (success) { | ||
| getContainerManager().notifyContainerReportProcessing(false, true); | ||
| } else { |
There was a problem hiding this comment.
Any reason we are not doing getContainerManager().notifyContainerReportProcessing(false, success) ?
There was a problem hiding this comment.
We can do that, just due to copy/paste from ICR code it is like this. (End result is same)
Let me know if you want to change it?
There was a problem hiding this comment.
IMO, we should change it to 1 line instead of if-else.
There was a problem hiding this comment.
As discussed offline, will change this in later jira. For now we shall go with the current way.
If the user wants to know numContainerReportsProcessed successfully in system. And he wants to compare with failure count, then it might be useful. And also I got to know yesterday it would be good to have metrics in the system for some actions happened/performed. |
|
Test failures are not related to this patch. |
No description provided.