-
Notifications
You must be signed in to change notification settings - Fork 501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDDS-10309. Speed up TestSnapshotDeletingService #6403
Conversation
Can this be reviewed please. |
There was a problem hiding this 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.
Is it possible to create a separate bucket for each of the tests? Rather than using the order of tests.
@aswinshakil please take a look as well.
@hemantk-12 , I tried removing order but it involves lot of cleaning of data and/or considering previous records for comparison and hence the code complexity will get increased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me. Left some minor comments.
Can we avoid order if we create a new bucket for each test?
@@ -81,8 +90,11 @@ public class TestSnapshotDeletingService { | |||
private static final String VOLUME_NAME = "vol1"; | |||
private static final String BUCKET_NAME_ONE = "bucket1"; | |||
private static final String BUCKET_NAME_TWO = "bucket2"; | |||
private static final String BUCKET_NAME_FSO = "bucketfso"; | |||
|
|||
private boolean runIndividualTest = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need runIndividualTest
if tests are running in order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By this check, I am making sure that the If anyone just want to run a single method it shall work for him.
//clean data | ||
// // cleaning up the data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can you please fix the duplicate comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. thanks.
What changes were proposed in this pull request?
Speed up TestSnapshotDeletingService
The time span is improved to 250.118 seconds from 321.147 seconds earlier.
https://github.com/raju-balpande/apache_ozone/actions/runs/8339044173/job/22820842037
Please describe your PR in detail:
We have managed the cluster creation part to be executed once.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10309?
How was this patch tested?
Tested each method independently and in sequence locally and Tested the sequence in Ci https://github.com/raju-balpande/apache_ozone/actions/runs/8339044173/job/22820842037