Skip to content

HDDS-11865. Remove tests for non-Ratis OM#7535

Merged
adoroszlai merged 9 commits intoapache:masterfrom
adoroszlai:HDDS-11865
Jan 1, 2025
Merged

HDDS-11865. Remove tests for non-Ratis OM#7535
adoroszlai merged 9 commits intoapache:masterfrom
adoroszlai:HDDS-11865

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

  1. Remove tests for non-Ratis OM (where we have similar test for with Ratis enabled) + TestOMEpochForNonRatis
  2. Enable OM Ratis in some tests where it was disabled
  3. Disable test cases in OM unit tests where test implementation is specific to non-Ratis mode (should be updated in HDDS-11868, HDDS-11869)
  4. Disable two test cases in TestHSync which fails with Ratis OM (should be fixed in HDDS-11870)
  5. Remove unnecessary "Enable OM Ratis" config update from integration tests

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

How was this patch tested?

CI:
https://github.com/adoroszlai/ozone/actions/runs/12196695873

@adoroszlai adoroszlai added the test label Dec 6, 2024
@adoroszlai adoroszlai self-assigned this Dec 6, 2024
@adoroszlai adoroszlai added om code-cleanup Changes that aim to make code better, without changing functionality. labels Dec 6, 2024
@swamirishi swamirishi requested a review from jojochuang December 9, 2024 16:44
@errose28
Copy link
Contributor

Thanks for working on this @adoroszlai. We also have non-ha and om-ha (no scm ha) clusters in the upgrade acceptance suite used for testing older versions. Should we remove those as well? Maybe in a follow up PR if that's easier.

@adoroszlai
Copy link
Contributor Author

We also have non-ha and om-ha (no scm ha) clusters in the upgrade acceptance suite used for testing older versions. Should we remove those as well? Maybe in a follow up PR if that's easier.

Thanks for mentioning these. Created HDDS-11902 for the follow-up.

}

@Test
@Unhealthy("HDDS-11870")
Copy link
Contributor

Choose a reason for hiding this comment

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

this is failing due to open key table not cleaned up properly. Given that it is a flaky test, it sounds to me the issue is probably double flush not flushed. So not a fundamental flaw.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

Had a quick pass. Just a few comments the rest is good.

/**
* Test Ozone Client with OM Ratis enabled.
*/
class TestOzoneRpcClientWithRatis extends OzoneRpcClientTests {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is replaced by TestOzoneRpcClient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry, forgot to mention some "WithRatis" tests were renamed after deleting the non-Ratis counterparts (name without classifier).

/**
* Tests the AWS S3 SDK basic operations with OM Ratis enabled.
*/
public class TestS3SDKV1WithRatis extends AbstractS3SDKV1Tests {
Copy link
Contributor

Choose a reason for hiding this comment

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

replaced by TestS3SDKV1

* Tests OM epoch generation for when Ratis is not enabled.
*/
@Timeout(240)
public class TestOMEpochForNonRatis {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a similar test when Ratis is enabled? Or is that not relevent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think transaction index comes from Ratis in that case.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

All tests pass. I think this one is good to go.

@adoroszlai adoroszlai merged commit 14756bf into apache:master Jan 1, 2025
@adoroszlai adoroszlai deleted the HDDS-11865 branch January 1, 2025 08:22
@adoroszlai
Copy link
Contributor Author

Thanks @jojochuang for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-cleanup Changes that aim to make code better, without changing functionality. om test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants