dombizita commented on code in PR #3837:
URL: https://github.com/apache/ozone/pull/3837#discussion_r996774041
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/api/db.json:
##########
@@ -1260,5 +1260,334 @@
"lastUpdatedTimestamp": 1663421094507,
"lastUpdatedSeqNumber": 0
}
- ]
+ ],
+ "UNDER_REPLICATED": {
+ "missingCount": 0,
+ "underReplicatedCount": 1,
+ "overReplicatedCount": 0,
+ "misReplicatedCount": 0,
+ "containers": [
+ {
+ "containerID": 2,
+ "containerState": "UNDER_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 3,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 1,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 2,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_UnderReplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_underreplicated2",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_underreplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ },
+ {
+ "containerID": 3,
+ "containerState": "UNDER_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 4,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 2,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 3,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_underreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_underreplicated3",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_underreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ }
+ ]
+ },
+ "Over_REPLICATED": {
Review Comment:
I think `db.json` won't work with this misspell.
```suggestion
"OVER_REPLICATED": {
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -35,18 +36,48 @@ interface IMissingContainerResponse {
pipelineID: string;
}
+interface IAllContainerResponse {
+ containerID: number;
+ containerState: string;
+ unhealthySince: string;
+ expectedReplicaCount: number;
+ actualReplicaCount: number;
+ replicaDeltaCount: number;
+ reason: string;
+ keys: number;
+ pipelineID: string;
+ replicas: IContainerReplicaAll[];
+}
+
export interface IContainerReplica {
containerId: number;
datanodeHost: string;
firstReportTimestamp: number;
lastReportTimestamp: number;
}
+export interface IContainerReplicaAll {
Review Comment:
```suggestion
export interface IContainerReplicas {
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -158,6 +189,294 @@ const KEY_TABLE_COLUMNS = [
}
];
+const UCOLUMNS = [
Review Comment:
I checked and the `UCOLUMNS`, `OCOLUMNS`, `MCOLUMNS` and `ACOLUMNS` are
exactly the same. Why do we have it copied 4 times?
If we want to handle the `MISSING` state the same as the others we could
have only the COLUMNS (starts on line 97) and adjust it to the parameters.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/api/db.json:
##########
@@ -1260,5 +1260,334 @@
"lastUpdatedTimestamp": 1663421094507,
"lastUpdatedSeqNumber": 0
}
- ]
+ ],
+ "UNDER_REPLICATED": {
+ "missingCount": 0,
+ "underReplicatedCount": 1,
+ "overReplicatedCount": 0,
+ "misReplicatedCount": 0,
+ "containers": [
+ {
+ "containerID": 2,
+ "containerState": "UNDER_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 3,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 1,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 2,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_UnderReplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_underreplicated2",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_underreplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ },
+ {
+ "containerID": 3,
+ "containerState": "UNDER_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 4,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 2,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 3,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_underreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_underreplicated3",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_underreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ }
+ ]
+ },
+ "Over_REPLICATED": {
+ "missingCount": 0,
+ "underReplicatedCount": 1,
+ "overReplicatedCount": 0,
+ "misReplicatedCount": 0,
+ "containers": [
+ {
+ "containerID": 2,
+ "containerState": "OVER_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 3,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 1,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 2,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_overreplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_overreplicated22",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_overreplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ },
+ {
+ "containerID": 3,
+ "containerState": "OVER_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 4,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 2,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 3,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_overreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_overreplicated3",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_overreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ }
+ ]
+ },
+ "MIS_REPLICATED": {
+ "missingCount": 0,
+ "underReplicatedCount": 1,
+ "overReplicatedCount": 0,
+ "misReplicatedCount": 0,
+ "containers": [
+ {
+ "containerID": 2,
+ "containerState": "MIS_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 3,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 1,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 2,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_misreplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_misreplicated2",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_misreplicated2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ },
+ {
+ "containerID": 3,
+ "containerState": "MIS_REPLICATED",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 4,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 2,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 3,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_misreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_misreplicated3",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_misreplicated3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ }
+ ]
+ },
+ "ALL_REPLICAS_UNHEALTHY": {
+ "missingCount": 0,
+ "underReplicatedCount": 1,
+ "overReplicatedCount": 1,
+ "misReplicatedCount": 1,
+ "containers": [
+ {
+ "containerID": 2,
+ "containerState": "ALL_REPLICAS_UNHEALTHY",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 3,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 1,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 2,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_allreplica2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_allreplica2",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 2,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_allreplica2",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ },
+ {
+ "containerID": 3,
+ "containerState": "ALL_REPLICAS_UNHEALTHY",
+ "unhealthySince": 1665590446222,
+ "expectedReplicaCount": 4,
+ "actualReplicaCount": 2,
+ "replicaDeltaCount": 2,
+ "reason": null,
+ "keys": 1,
+ "pipelineID": "a10ffab6-8ed5-414a-aaf5-79890ff3e8a1",
+ "replicas": [
+ {
+ "containerId": 3,
+ "datanodeUuid": "15526f1b-76f2-4d8f-876c-c343c94ea476",
+ "datanodeHost": "ozone_datanode_2.ozone_allreplica3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590397315,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "f55476ab-4687-464d-a100-1c65de4366e3",
+ "datanodeHost": "ozone_datanode_3.ozone_allreplica3",
+ "firstSeenTime": 1665588176616,
+ "lastSeenTime": 1665590392293,
+ "lastBcsId": 2
+ },
+ {
+ "containerId": 3,
+ "datanodeUuid": "7a457bcb-d63e-49cc-b3ff-8b22bf48d130",
+ "datanodeHost": "ozone_datanode_1.ozone_allreplica3",
+ "firstSeenTime": 1665588176660,
+ "lastSeenTime": 1665590272289,
+ "lastBcsId": 0
+ }
+ ]
+ }
+ ]
+ }
+
Review Comment:
In this PR we shouldn't integrate with the state `ALL_REPLICA_UNHEALTHY` as
the data is not populated correctly yet on the Recon side. I think we should
remove code related to that and add it only if it is working correctly.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -192,16 +519,41 @@ export class MissingContainers extends
React.Component<Record<string, object>, I
this.setState({
loading: true
});
- axios.get('/api/v1/containers/missing').then(response => {
- const missingContainersResponse: IMissingContainersResponse =
response.data;
- const totalCount = missingContainersResponse.totalCount;
- const missingContainers: IMissingContainerResponse[] =
missingContainersResponse.containers;
+
+ axios.all([
+ axios.get('/api/v1/containers/missing'),
+ axios.get('/api/v1/containers/unhealthy/UNDER_REPLICATED'),
+ axios.get('/api/v1/containers/unhealthy/OVER_REPLICATED'),
+ axios.get('/api/v1/containers/unhealthy/MIS_REPLICATED'),
+ axios.get('/api/v1/containers/unhealthy/ALL_REPLICAS_UNHEALTHY'),
+ ]).then(axios.spread((missingContainersResponse, underReplicatedResponse,
overReplicatedResponse, misReplicatedResponse, allReplicatedResponse) => {
+
+ const missingContainersResponse1: IMissingContainersResponse =
missingContainersResponse.data;
Review Comment:
Why do we need to number (eg. `missingContainersResponse1`) the variables
like this? I don't see other usages later.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -213,6 +565,13 @@ export class MissingContainers extends
React.Component<Record<string, object>, I
console.log(current, pageSize);
};
+ onTabChange = (activeKey: string) => {
+ // Fetch inactive pipelines if tab is switched to "Inactive"
+ if (activeKey === '2') {
+ // Fetch inactive pipelines in the future
+ }
+ };
+
Review Comment:
I don't understand this part and the comments. How do pipelines come here?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -35,18 +36,48 @@ interface IMissingContainerResponse {
pipelineID: string;
}
+interface IAllContainerResponse {
Review Comment:
Some naming suggestions, maybe it is more clear what are these used for.
```suggestion
interface IContainerResponse {
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -82,7 +113,7 @@ const COLUMNS = [
key: 'replicas',
render: (replicas: IContainerReplica[]) => (
<div>
- {replicas.map(replica => {
+ {replicas && replicas.map(replica => {
Review Comment:
Can you help me understand why do we need this?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/missingContainers/missingContainers.tsx:
##########
@@ -35,18 +36,48 @@ interface IMissingContainerResponse {
pipelineID: string;
}
+interface IAllContainerResponse {
+ containerID: number;
+ containerState: string;
+ unhealthySince: string;
+ expectedReplicaCount: number;
+ actualReplicaCount: number;
+ replicaDeltaCount: number;
+ reason: string;
+ keys: number;
+ pipelineID: string;
+ replicas: IContainerReplicaAll[];
+}
+
export interface IContainerReplica {
containerId: number;
datanodeHost: string;
firstReportTimestamp: number;
lastReportTimestamp: number;
}
+export interface IContainerReplicaAll {
+ containerId: number;
+ datanodeUuid: string;
+ datanodeHost: string;
+ firstSeenTime: number;
+ lastSeenTime: number;
+ lastBcsId: number;
+}
+
export interface IMissingContainersResponse {
totalCount: number;
containers: IMissingContainerResponse[];
}
+interface IAllReplicatedContainersResponse {
Review Comment:
```suggestion
interface IUnhealthyContainersResponse {
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/api/routes.json:
##########
@@ -23,5 +23,9 @@
"/namespace/du?path=/clunky&files=true": "/clunky",
"/namespace/summary?path=*": "/metadata",
"/namespace/quota?path=*": "/quota",
- "/task/status": "/taskStatus"
+ "/task/status": "/taskStatus",
+ "/containers/unhealthy/UNDER_REPLICATED": "/UNDER_REPLICATED",
+ "/containers/unhealthy/OVER_REPLICATED": "/OVER_REPLICATED",
+ "/containers/unhealthy/MIS_REPLICATED": "/MIS_REPLICATED",
+ "/containers/unhealthy/ALL_REPLICAS_UNHEALTHY": "/ALL_REPLICAS_UNHEALTHY"
Review Comment:
On the line 4 there is one route for the missing containers, maybe we should
follow this pattern and add it like this (and remove the previous one). Also
remove the route to `ALL_REPLICAS_UNHEALTHY`.
```suggestion
"/containers/unhealthy/MISSING": "/missingContainers",
"/containers/unhealthy/UNDER_REPLICATED": "/underreplicatedContainers",
"/containers/unhealthy/OVER_REPLICATED": "/overreplicatedContainers",
"/containers/unhealthy/MIS_REPLICATED": "/misreplicatedContainers"
```
Maybe we can consider to add the tag `@Deprecated` to the /missing endpoint
in ContainerEndpoint. Let me know what you think @smengcl
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
