Skip to content

Conversation

@aswinshakil
Copy link
Member

What changes were proposed in this pull request?

#7140 Added a framework to add metrics for any AbstractReplicationTask. This adds all the necessary metrics required for container reconciliation task(ReconcileContainerTask). This JIRA aims to add tests for these metrics.

What is the link to the Apache JIRA

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

How was this patch tested?

Added unit tests.

Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test @aswinshakil. Can we move this so the tests for reconciliation tasks are in their own test method within TestReplicationSupervisor? We can throw one into the mix in testMultipleReplication since it is already there, but that method is already testing too many things at once and it is hard to follow the individual things being checked.

Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for updating this @aswinshakil

@errose28
Copy link
Contributor

We need #7699 in our branch to fix acceptance tests

Copy link
Contributor

@errose28 errose28 left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @aswinshakil. Let's get the reverse merge from master in to fix our CI, then we can start merging our approved PRs to the branch.

@errose28
Copy link
Contributor

errose28 commented Feb 7, 2025

@aswinshakil The reverse merge from master is done with green CI. Please merge this into your PR branch to retrigger CI with the new changes, which should fix the acceptance test issues.

@kerneltime kerneltime merged commit fa8e0a4 into apache:HDDS-10239-container-reconciliation Feb 10, 2025
27 of 42 checks passed
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.

3 participants