Delete runs in daterange in batches #433

Roemer · 2021-09-07T15:06:48Z

This PR changes the behavior of the deletion of runs in a specified date range.
Previously, it used a very slow aggregate to find the instances for the runs that should be deleted and tried to delete them all at once. This timeouted at various ends in larger setups.
With this change, it will now process the deletion in batches:

Get a batch of runIds
Delete all associated instances for those runIds
Delete the runs for those runIds
Repeat until no more runs are found for the defined date-range

To make it even faster, a new index for runId is added to the instance collection.

agoldis

Great job @Roemer! Thanks a lot for fixing that code I am happy to see that people use Sorry Cypress to run as many test that we are facing performance issues 😬

Just a few minor comments:

batched iteration on results is great and simple! I think the same result could be achieved with native mongoDB cursor, but your implementation is totally find!
if we are really looking to optimize it, we can limit the fields we get from getRunsInDateRange by explicitly fetching only runIds. If I am not mistaken, mongoDB won't even read data from the disk and send the results back straight from the runId index on runs collection.

agoldis · 2021-09-08T06:22:57Z

packages/api/src/datasources/runs.ts

+  async deleteRunsInDateRange(
+    startDate: Date,
+    endDate: Date,
+    limit: number = 0
+  ) {
+    const getResult = await this.getRunsInDateRange(startDate, endDate, limit);
+    return this.deleteRunsByIds(getResult.runIds);
+  }


This one is not used, please delete

agoldis · 2021-09-08T06:31:02Z

packages/api/src/datasources/instances.ts

-
-    const runIds = response.map((x) => x.run[0].runId) as string[];
-    return await this.deleteInstancesByRunIds(runIds);
-  }


agoldis · 2021-09-08T06:31:34Z

packages/api/src/resolvers/index.ts

+          break;
+        }
+        // Delete the runs
+        const runsDeleteResponse = await resolvers.Mutation.deleteRuns(


Nice reuse of mutation 👍🏻

agoldis · 2021-09-08T06:39:24Z

P.S. I've merged #427 into master and updated this branch, don't be scared

Roemer · 2021-09-08T07:59:17Z

I rebased it back to master instead of merged and removed the unneeded method and added a projection when getting the runs.
Good point about the cursor. I had a look at it and it would not really fit well into the current architecture. We would need to add the cursor to datasources/run.ts and actually return it so that we can handle the deletion of the instances, runs and runtimeouts in the mutation itself. But then we would have a datasource related object (the cursor) in the mutation itself which seems wrong. Or we would pass a function to the run datasource which would handle that all, but that also seems not so nice. So I think for a first version, we can go with this.
Testing is still pending on my side. I will probably need to do that tomorrow.

Roemer · 2021-09-09T08:24:18Z

I now tested it and I could delete 2 months worth of data in a few seconds ;) So from my side, this PR can be merged.

agoldis approved these changes Sep 8, 2021

View reviewed changes

agoldis reviewed Sep 8, 2021

View reviewed changes

Roman Bäriswyl and others added 3 commits September 8, 2021 09:40

Delete runs in daterange in batches

d2b3665

Create an index for runId on instance

e0362ab

Delete unused method, limit the projection when getting runs

5c6a202

Roemer force-pushed the feature/CleanupInBatches branch from a2f4b95 to 5c6a202 Compare September 8, 2021 07:52

Roemer force-pushed the feature/CleanupInBatches branch from 6694160 to 5c6a202 Compare September 9, 2021 08:22

agoldis merged commit c700d89 into sorry-cypress:master Sep 9, 2021

Roemer deleted the feature/CleanupInBatches branch September 14, 2021 11:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete runs in daterange in batches #433

Delete runs in daterange in batches #433

Roemer commented Sep 7, 2021 •

edited

agoldis left a comment

agoldis Sep 8, 2021

agoldis Sep 8, 2021

agoldis Sep 8, 2021

agoldis commented Sep 8, 2021 •

edited

Roemer commented Sep 8, 2021

Roemer commented Sep 9, 2021

Delete runs in daterange in batches #433

Delete runs in daterange in batches #433

Conversation

Roemer commented Sep 7, 2021 • edited

agoldis left a comment

Choose a reason for hiding this comment

agoldis Sep 8, 2021

Choose a reason for hiding this comment

agoldis Sep 8, 2021

Choose a reason for hiding this comment

agoldis Sep 8, 2021

Choose a reason for hiding this comment

agoldis commented Sep 8, 2021 • edited

Roemer commented Sep 8, 2021

Roemer commented Sep 9, 2021

Roemer commented Sep 7, 2021 •

edited

agoldis commented Sep 8, 2021 •

edited