You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smitajoshi12 (via GitHub)" <gi...@apache.org> on 2023/04/10 17:23:44 UTC

[GitHub] [ozone] smitajoshi12 opened a new pull request, #4555: HDDS-8340. Recon Show the number of keys marked for Deletion in Recon UI.

smitajoshi12 opened a new pull request, #4555:
URL: https://github.com/apache/ozone/pull/4555

   
   ## What changes were proposed in this pull request?
   This information will be displayed on the overview page of recon which gets populated with the information provided by the ClusterStateEndpoint
   
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-8340
   
   Please replace this section with the link to the Apache JIRA)
   
   ## How was this patch tested?
   Manually
   ![image](https://user-images.githubusercontent.com/112169209/230955683-a5b2d421-79a6-4535-99a7-ab8396760a4d.png)
   
   


-- 
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


[GitHub] [ozone] smengcl merged pull request #4555: HDDS-8340. [Recon] Show the number of deleted containers in UI

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #4555:
URL: https://github.com/apache/ozone/pull/4555


-- 
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


[GitHub] [ozone] kerneltime commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1511670173

   @devmadhuu @xBis7 can you'll please take a look?


-- 
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


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #4555:
URL: https://github.com/apache/ozone/pull/4555#discussion_r1178688155


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/overview/overview.tsx:
##########
@@ -227,6 +231,9 @@ export class Overview extends React.Component<Record<string, object>, IOverviewS
           <Col xs={24} sm={18} md={12} lg={12} xl={6}>
             <OverviewCard loading={loading} title='Keys' data={keys.toString()} icon='file-text'/>
           </Col>
+          <Col xs={24} sm={18} md={12} lg={12} xl={6}>
+            <OverviewCard loading={loading} title='Deleted Containers' data={deletedContainers !== undefined && deletedContainers.toString()} icon='delete' />

Review Comment:
   @dombizita 
   Removed undefined condition in latest commit.



-- 
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


[GitHub] [ozone] xBis7 commented on a diff in pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #4555:
URL: https://github.com/apache/ozone/pull/4555#discussion_r1170341845


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/overview/overview.tsx:
##########
@@ -227,6 +231,9 @@ export class Overview extends React.Component<Record<string, object>, IOverviewS
           <Col xs={24} sm={18} md={12} lg={12} xl={6}>
             <OverviewCard loading={loading} title='Keys' data={keys.toString()} icon='file-text'/>
           </Col>
+          <Col xs={24} sm={18} md={12} lg={12} xl={6}>
+            <OverviewCard loading={loading} title='Deleted Containers' data={deletedContainers !== undefined && deletedContainers.toString()} icon='delete' />

Review Comment:
   This check seems redundant `deletedContainers !== undefined`, unless there is a chance that the server response won't contain the field.



-- 
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


[GitHub] [ozone] smitajoshi12 commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1512562662

   > IMHO, displaying the number of missing containers would be much more useful than the number of deleted containers. Does it make more sense to see how many of the existing containers have gone missing or are unhealthy? Or how many data you didn't need and eventually deleted?
   
   @xBis7 
   Missing Containers functionality is already implemented in overview page.
   
   ![image](https://user-images.githubusercontent.com/112169209/232698358-3f18594e-db3c-480c-9368-56ce7dad159c.png)
   


-- 
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


[GitHub] [ozone] xBis7 commented on a diff in pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #4555:
URL: https://github.com/apache/ozone/pull/4555#discussion_r1170341845


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/overview/overview.tsx:
##########
@@ -227,6 +231,9 @@ export class Overview extends React.Component<Record<string, object>, IOverviewS
           <Col xs={24} sm={18} md={12} lg={12} xl={6}>
             <OverviewCard loading={loading} title='Keys' data={keys.toString()} icon='file-text'/>
           </Col>
+          <Col xs={24} sm={18} md={12} lg={12} xl={6}>
+            <OverviewCard loading={loading} title='Deleted Containers' data={deletedContainers !== undefined && deletedContainers.toString()} icon='delete' />

Review Comment:
   This check seems redundant `deletedContainers !== undefined`, unless there is a chance that the server response won't contain it.



-- 
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


[GitHub] [ozone] devmadhuu commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1513459741

   > > I think Recon displays the missing containers and unhealthy containers under "Containers" tab. This PR is mainly focussed to show count on Overview page.
   > 
   > @devmadhuu I know and also there is the number of containers next to each tab header (missing, under-replicated, ...). Why would the user want to see the number of deleted containers?
   
   @xBis7 as far as i know , this count of deleted containers was done based on multiple issues being faced while debugging customer issues, so needed this info to correlate with information in rocks db. Mainly targeted for admin users not for direct customers.


-- 
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


[GitHub] [ozone] xBis7 commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1513486726

   > this count of deleted containers was done based on multiple issues being faced while debugging customer issues, so needed this info to correlate with information in rocks db. Mainly targeted for admin users not for direct customers.
   
   @devmadhuu Thanks for the info. Based on that, this PR looks good. 
   
   CI failure seems unrelated.


-- 
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


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #4555:
URL: https://github.com/apache/ozone/pull/4555#discussion_r1173809475


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/overview/overview.tsx:
##########
@@ -227,6 +231,9 @@ export class Overview extends React.Component<Record<string, object>, IOverviewS
           <Col xs={24} sm={18} md={12} lg={12} xl={6}>
             <OverviewCard loading={loading} title='Keys' data={keys.toString()} icon='file-text'/>
           </Col>
+          <Col xs={24} sm={18} md={12} lg={12} xl={6}>
+            <OverviewCard loading={loading} title='Deleted Containers' data={deletedContainers !== undefined && deletedContainers.toString()} icon='delete' />

Review Comment:
   @xBis7
   I checked with docker also working fine with undefined condition.
   undefined is additional check if deletedContainers will fail in API Response.



-- 
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


[GitHub] [ozone] dombizita commented on a diff in pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #4555:
URL: https://github.com/apache/ozone/pull/4555#discussion_r1177628455


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/overview/overview.tsx:
##########
@@ -227,6 +231,9 @@ export class Overview extends React.Component<Record<string, object>, IOverviewS
           <Col xs={24} sm={18} md={12} lg={12} xl={6}>
             <OverviewCard loading={loading} title='Keys' data={keys.toString()} icon='file-text'/>
           </Col>
+          <Col xs={24} sm={18} md={12} lg={12} xl={6}>
+            <OverviewCard loading={loading} title='Deleted Containers' data={deletedContainers !== undefined && deletedContainers.toString()} icon='delete' />

Review Comment:
   I am also not sure why the `deletedContainers !== undefined` check is needed, non of the above `OverviewCard`s contain it.
   why can't we assume that the API response will always return this information? the backend part of this have been in done in #4440 and I don't see anything there that would require this check, right @ArafatKhan2198? 
   btw I tried in a docker cluster and it was working for me.



-- 
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


[GitHub] [ozone] xBis7 commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1511733453

   IMHO, displaying the number of missing containers would be much more useful than the number of deleted containers. Does it make more sense to see how many of the existing containers have gone missing or are unhealthy? Or how many data you didn't need and eventually deleted?


-- 
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


[GitHub] [ozone] smengcl commented on pull request #4555: HDDS-8340. [Recon] Show the number of deleted containers in UI

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1535186750

   Thanks @smitajoshi12 for the addition. Thanks @xBis7 @devmadhuu @dombizita for the reviews.


-- 
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


[GitHub] [ozone] devmadhuu commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1513337198

   > IMHO, displaying the number of missing containers would be much more useful than the number of deleted containers. Does it make more sense to see how many of the existing containers have gone missing or are unhealthy? Or how many data you didn't need and eventually deleted?
   
   @xBis7 Thanks for your review, I think Recon displays the missing containers and unhealthy containers under "Containers" tab. This PR is mainly focussed to show count on Overview page.


-- 
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


[GitHub] [ozone] xBis7 commented on pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on PR #4555:
URL: https://github.com/apache/ozone/pull/4555#issuecomment-1513438457

   > I think Recon displays the missing containers and unhealthy containers under "Containers" tab. This PR is mainly focussed to show count on Overview page.
   
   @devmadhuu I know and also there is the number of containers on each tab(missing, under-replicated, ...). Why would the user want to see the number of deleted containers?


-- 
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


[GitHub] [ozone] xBis7 commented on a diff in pull request #4555: HDDS-8340. Recon Show the number of containers that have been DELETED in UI.

Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #4555:
URL: https://github.com/apache/ozone/pull/4555#discussion_r1170344334


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/overview/overview.tsx:
##########
@@ -227,6 +231,9 @@ export class Overview extends React.Component<Record<string, object>, IOverviewS
           <Col xs={24} sm={18} md={12} lg={12} xl={6}>
             <OverviewCard loading={loading} title='Keys' data={keys.toString()} icon='file-text'/>
           </Col>
+          <Col xs={24} sm={18} md={12} lg={12} xl={6}>
+            <OverviewCard loading={loading} title='Deleted Containers' data={deletedContainers !== undefined && deletedContainers.toString()} icon='delete' />

Review Comment:
   Can you do a manual test with docker, without the `pnpm` tool?



-- 
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