You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/17 22:29:50 UTC

[GitHub] [ozone] dombizita commented on a diff in pull request #3837: HDDS-7248. Recon: Expand the container status page to show all unhealthy container states

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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org