Skip to content

[backend] Various performance improvments #10971

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

richard-julien
Copy link
Member

Various performance improvments

  • Usage of filter instead of direct bool to prevent scoring computing
  • Prevent ordering if not needed
  • Prevent usage track_total_hits if not needed
  • Implement a generic asyncMap with optional transform to prevent event loop blocking
  • Add a buffer time in redis stream listening to prevent too much call and use bulk execution

Copy link

codecov bot commented May 11, 2025

Codecov Report

Attention: Patch coverage is 33.84615% with 43 lines in your changes missing coverage. Please review.

Project coverage is 65.61%. Comparing base (5efa321) to head (8cd1dae).
Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
...tform/opencti-graphql/src/utils/data-processing.ts 5.55% 17 Missing ⚠️
...orm/opencti-graphql/src/graphql/telemetryPlugin.js 0.00% 9 Missing ⚠️
...cti-platform/opencti-graphql/src/database/redis.ts 0.00% 8 Missing ⚠️
...ti-platform/opencti-graphql/src/database/engine.js 86.36% 3 Missing ⚠️
...i-platform/opencti-graphql/src/domain/connector.ts 0.00% 2 Missing ⚠️
...orm/opencti-graphql/src/manager/activityManager.ts 0.00% 1 Missing ⚠️
...rm/opencti-graphql/src/manager/fileIndexManager.ts 0.00% 1 Missing ⚠️
...form/opencti-graphql/src/manager/historyManager.ts 0.00% 1 Missing ⚠️
...rm/opencti-graphql/src/manager/publisherManager.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10971      +/-   ##
==========================================
+ Coverage   65.48%   65.61%   +0.13%     
==========================================
  Files         674      674              
  Lines       67138    67182      +44     
  Branches     7383     7425      +42     
==========================================
+ Hits        43962    44080     +118     
+ Misses      23176    23102      -74     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@@ -3059,9 +3069,7 @@ const elQueryBodyBuilder = async (context, user, options) => {
}
}
// Add standard_id if not specify to ensure ordering uniqueness
Copy link
Member

Choose a reason for hiding this comment

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

So this comment is not relevant anymore?

Comment on lines +3083 to +3084
} else { // If not ordering criteria, order by _doc
ordering.push({ _doc: 'asc' });
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure to understand this else case. We are doing the same thing we did line 3072, so is it useful?

Copy link
Member

Choose a reason for hiding this comment

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

I guess it could be simplified by adding ordering.push({ _doc: 'asc' }); all the time, by moving line 3072 to 3084 and removing the else condition.

@richard-julien richard-julien added the filigran team use to identify PR from the Filigran team label May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants