Skip to content

Conversation

@targetoee
Copy link
Contributor

What changes were proposed in this pull request?

This PR adds additional container information to Recon, which shows the total count of containers in some states, including CLOSING, QUASI_CLOSED, CLOSED, and DELETING.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8411

How was this patch tested?

Unit test
Screen Shot 2023-04-19 at 4 20 44 PM

@dombizita dombizita changed the title add transitional container count in recon HDDS-8411. Show the total number of CLOSING, QUASI_CLOSED, CLOSED, DELETING Containers in Recon Apr 19, 2023
@devmadhuu
Copy link
Contributor

@targetoee - Thanks for working on this patch, however transitioning of container states could be very fast in a busy cluster, and not sure how much this info will be useful to show CLOSING, QUASI_CLOSED and DELETING containers count. We show OPEN container count already and total container count, so difference will give us CLOSED container count.

.setClosingContainers(containerStateCounts.getClosingContainersCount())
.setQuasiClosedContainers(
containerStateCounts.getQuasiClosedContainersCount())
.setClosedContainers(containerStateCounts.getClosedContainersCount())
Copy link
Contributor

Choose a reason for hiding this comment

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

This may not be needed, as difference of total and open will give us closed container count. Rest all information on container states can be easily found out using Ozone admin CLI because we don't want to expose the transitioning container states to other users, and should be exposed to Admin users only.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @devmadhuu
@sumitagrawl has suggested that we include additional states in our state counts, such as close, closing, and quasi_closed. The state closing is especially important and can provide valuable insights into the situation being analyzed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ArafatKhan2198 , these state transitions are too dynamic and quick, let's discuss this, not sure if in so dynamic transitions, if it is really useful for admins. Moreover these are directly queried using admin CLI, so better we should avoid such data and load recon

Copy link
Contributor

Choose a reason for hiding this comment

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

Also closed is not needed because it can be derived from difference

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, Quasi-Closed can be important state having special meaning. Also Closing / Deleting if stuck can be identified easily from this perspective.

Choose a reason for hiding this comment

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

Yes in normal scenarios transition will be quick, but have seen large number of containers stuck and remaining in Closing state in some scenarios. So should be useful for analyzing, if easily visible. But we should be careful of cluttering overview page and should have at appropriate place.

.setClosedContainers(containerStateCounts.getClosedContainersCount())
.setDeletingContainers(
containerStateCounts.getDeletingContainersCount())
.setDeletedContainers(containerStateCounts.getDeletedContainersCount())
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be duplicate of PR for HDDS-8127

Copy link
Contributor

Choose a reason for hiding this comment

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

Deleted Container and Deleting container shows different info ... and not to be excluded from container count. Only Deleted Container needs to be excluded,

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@ArafatKhan2198 Thanks for working on this, plz check my opinion

.setClosingContainers(containerStateCounts.getClosingContainersCount())
.setQuasiClosedContainers(
containerStateCounts.getQuasiClosedContainersCount())
.setClosedContainers(containerStateCounts.getClosedContainersCount())
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, Quasi-Closed can be important state having special meaning. Also Closing / Deleting if stuck can be identified easily from this perspective.

.setClosedContainers(containerStateCounts.getClosedContainersCount())
.setDeletingContainers(
containerStateCounts.getDeletingContainersCount())
.setDeletedContainers(containerStateCounts.getDeletedContainersCount())
Copy link
Contributor

Choose a reason for hiding this comment

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

Deleted Container and Deleting container shows different info ... and not to be excluded from container count. Only Deleted Container needs to be excluded,

@adoroszlai
Copy link
Contributor

/pending

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

/pending

@github-actions
Copy link

github-actions bot commented Feb 5, 2024

Thank you very much for the patch. I am closing this PR temporarily as there was no activity recently and it is waiting for response from its author.

It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.

It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.

If you need ANY help to finish this PR, please contact the community on the mailing list or the slack channel."

@github-actions github-actions bot closed this Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants