Skip to content

Conversation

@raju-balpande
Copy link
Contributor

What changes were proposed in this pull request?

Changes are done to improve the test run speed to 33.23 s as compare to 59.43 s earlier.

  • The setup method modified to be initialize one time for all methods.
  • Introduced the method orderer
  • Declared one counter to track the number of snapshots been generated in all the methods to assert the count according.
  • Separated the methods for creation of volume and bucket which was combine and there were repeated calls to it.
  • Changed one assert statement to assert that because the .sst file from the bucket created in first method was not consistent to available in second method.

What is the link to the Apache JIRA

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

How was this patch tested?

Tested on local system and in CI-CD integration.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @raju-balpande for the patch.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @raju-balpande for updating the patch, LGTM.

Copy link
Contributor

@myskov myskov left a comment

Choose a reason for hiding this comment

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

I would avoid tests ordering.
Instead of counting snapshots across these tests, I think it will be better to get the initial snapshot number in the beginning of each test:

long initialSnapshotsNum = sstFilteringService.getSnapshotFilteredCount().get();
...
waitForSnapshotsAtLeast(filteringService, initialSnapshotsNum + 1);

This will keep these tests independent.

@hemantk-12
Copy link
Contributor

I would avoid tests ordering.

+1 if it is possible.

Otherwise patch looks good to me.

@raju-balpande
Copy link
Contributor Author

The methods are independent of ordering flow when snapshots are concerned. Independent method run also works fine. but the ordering was required because checks on key operations are dependent. Clearing all the data again will consume time and there will be hardly any improvement in time. but I am trying to see if I can fix it for keys.

@adoroszlai adoroszlai merged commit cd00691 into apache:master Feb 12, 2024
@adoroszlai
Copy link
Contributor

adoroszlai commented Feb 12, 2024

Thanks @raju-balpande for the patch, @hemantk-12, @myskov for the review.

Let's work on improving independence of test methods separately (HDDS-10346).

xichen01 pushed a commit to xichen01/ozone that referenced this pull request Oct 1, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants