[Backend] Enhance retention policy deletion performances / speed (#4864) #8569

richard-julien · 2024-10-01T20:51:03Z

codecov · 2024-10-01T21:11:44Z

Codecov Report

Attention: Patch coverage is 28.88889% with 32 lines in your changes missing coverage. Please review.

Project coverage is 66.24%. Comparing base (cec4c9e) to head (1dce179).
Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
...rm/opencti-graphql/src/manager/retentionManager.ts	37.93%	18 Missing ⚠️
...tform/opencti-graphql/src/manager/managerModule.ts	10.00%	9 Missing ⚠️
...atform/opencti-graphql/src/domain/retentionRule.js	16.66%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8569      +/-   ##
==========================================
+ Coverage   66.22%   66.24%   +0.02%     
==========================================
  Files         597      597              
  Lines       60433    60428       -5     
  Branches     6202     6212      +10     
==========================================
+ Hits        40022    40032      +10     
+ Misses      20411    20396      -15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

labo-flg · 2024-10-02T07:12:36Z

opencti-platform/opencti-graphql/src/manager/retentionManager.ts

+        logApp.error(e, { id: node.id, manager: 'RETENTION_MANAGER' });
+      }
+    };
+    await BluePromise.map(elements, deleteFn, { concurrency: RETENTION_MAX_CONCURRENCY });


Just for my understanding: if we batch elements that are interdependent, the first one might take a lock on another one in the concurrent batch, and when release, the second element to delete might already be deleted (for instance if it's a relationship). Am I right ?

If so, not harmful but we'll end up with a bunch of error logs.

Add code to prevent logging for an already deleted error

labo-flg

Beside my comment on error logs, code seems ok to me !

Archidoit · 2024-10-02T09:01:10Z

I created a retention policy with no filters and with retention = 1 day. It should delete all the entities of my platform.
I have this error in backend every few seconds:

And if I restart my platform :

I think it's due to this modification :

Archidoit · 2024-10-02T09:04:54Z

opencti-platform/opencti-graphql/src/manager/retentionManager.ts

@@ -42,7 +45,7 @@ export const getElementsToDelete = async (context: AuthContext, scope: string, b
  if (scope === 'knowledge') {
    const jsonFilters = filters ? JSON.parse(filters) : null;
    const queryOptions = await convertFiltersToQueryOptions(jsonFilters, { before });
-    result = await elPaginate(context, RETENTION_MANAGER_USER, READ_STIX_INDICES, { ...queryOptions, first: RETENTION_BATCH_SIZE });
+    result = await elPaginate(context, RETENTION_MANAGER_USER, READ_ENTITIES_INDICES_WITHOUT_INFERRED, { ...queryOptions, first: RETENTION_BATCH_SIZE });


why this modification?

Nice catch, it was a test. Will rollback

retention rule also deletes internal objects

Archidoit · 2024-10-03T07:32:54Z

I created a retention policy on everything (Knowledge). It's indicating 512 elements will be deleting.

The retention rules is launched every 30s.
After 2 minutes nothing has been deleted. And when we 'test' the retention rule, the number of entities to be deleted is still the same :

-->entities have not been deleted

This didn't happen for a retention policy with a filter, and so targeting less elements (exemple done with 2 reports to delete: the reports where deleted).

Archidoit · 2024-10-03T07:34:09Z

The 'remaining' attribute seems not correct.
With this retention rule indicating 25 entities are 'remaining' while I only see 1 left :

Archidoit · 2024-10-03T12:36:02Z

Behavior ok 👍

Small question: With a retention rule on Knowledge with no filters, organizations are also deleted (because they are stix domain objects)... Is it a problem?

Archidoit · 2024-10-04T07:18:51Z

I don't really agree with the last commit that prevents empty filters for several reasons :

we avoid empty filters in UI but not in the API
there is no messages in the UI informing the Create button is disabled because filters are now mandatory
the problem is not really solved because if we add the filter: 'label different from label1' we will still delete the organization that haven't label1

--> I think that if we don't want to delete organizations we should think about it a bit more, maybe add a warning if a retention rule may delete organizations...

still problem

Archidoit · 2024-10-04T08:00:46Z

I don't really agree with the last commit that prevents empty filters for several reasons :

we avoid empty filters in UI but not in the API

there is no messages in the UI informing the Create button is disabled because filters are now mandatory

the problem is not really solved because if we add the filter: 'label different from label1' we will still delete the organization that haven't label1

--> I think that if we don't want to delete organizations we should think about it a bit more, maybe add a warning if a retention rule may delete organizations...

Since it is not the scope, I reverted the last commit and created an issue for the organization deletion issue : #8606

[Backend] Enhance retention policy deletion performances / speed (#4864)

9173cdc

github-actions bot added the filigran team use to identify PR from the Filigran team label Oct 1, 2024

richard-julien self-assigned this Oct 1, 2024

richard-julien linked an issue Oct 1, 2024 that may be closed by this pull request

Enhance retention policy deletion performances / speed #4864

Closed

richard-julien removed their assignment Oct 1, 2024

labo-flg reviewed Oct 2, 2024

View reviewed changes

labo-flg previously approved these changes Oct 2, 2024

View reviewed changes

Archidoit reviewed Oct 2, 2024

View reviewed changes

[Backend] Adapt logging and rollback impacted indices (#4864)

ceed742

[backend] Align indices in checkRetentionRule (#4864)

750d8ac

Archidoit previously approved these changes Oct 3, 2024

View reviewed changes

Archidoit force-pushed the issue/4864 branch from 1dce179 to 750d8ac Compare October 4, 2024 07:54

Archidoit approved these changes Oct 4, 2024

View reviewed changes

labo-flg merged commit 649af4f into master Oct 4, 2024
7 checks passed

labo-flg deleted the issue/4864 branch October 4, 2024 09:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Backend] Enhance retention policy deletion performances / speed (#4864) #8569

[Backend] Enhance retention policy deletion performances / speed (#4864) #8569

richard-julien commented Oct 1, 2024

codecov bot commented Oct 1, 2024 •

edited

Loading

labo-flg Oct 2, 2024 •

edited

Loading

richard-julien Oct 2, 2024

labo-flg left a comment

Archidoit commented Oct 2, 2024 •

edited

Loading

Archidoit Oct 2, 2024 •

edited

Loading

richard-julien Oct 2, 2024

richard-julien Oct 2, 2024

Archidoit commented Oct 3, 2024 •

edited

Loading

Archidoit commented Oct 3, 2024 •

edited

Loading

Archidoit commented Oct 3, 2024 •

edited

Loading

Archidoit commented Oct 4, 2024 •

edited

Loading

Archidoit commented Oct 4, 2024

[Backend] Enhance retention policy deletion performances / speed (#4864) #8569

[Backend] Enhance retention policy deletion performances / speed (#4864) #8569

Conversation

richard-julien commented Oct 1, 2024

codecov bot commented Oct 1, 2024 • edited Loading

Codecov Report

labo-flg Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

richard-julien Oct 2, 2024

Choose a reason for hiding this comment

labo-flg left a comment

Choose a reason for hiding this comment

Archidoit commented Oct 2, 2024 • edited Loading

Archidoit Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

richard-julien Oct 2, 2024

Choose a reason for hiding this comment

richard-julien Oct 2, 2024

Choose a reason for hiding this comment

Archidoit commented Oct 3, 2024 • edited Loading

Archidoit commented Oct 3, 2024 • edited Loading

Archidoit commented Oct 3, 2024 • edited Loading

Archidoit commented Oct 4, 2024 • edited Loading

Archidoit commented Oct 4, 2024

codecov bot commented Oct 1, 2024 •

edited

Loading

labo-flg Oct 2, 2024 •

edited

Loading

Archidoit commented Oct 2, 2024 •

edited

Loading

Archidoit Oct 2, 2024 •

edited

Loading

Archidoit commented Oct 3, 2024 •

edited

Loading

Archidoit commented Oct 3, 2024 •

edited

Loading

Archidoit commented Oct 3, 2024 •

edited

Loading

Archidoit commented Oct 4, 2024 •

edited

Loading