Skip to content
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

Internal DB enrichment performance improvements #11981

Merged
merged 12 commits into from
Oct 18, 2023

Conversation

mike12345567
Copy link
Collaborator

@mike12345567 mike12345567 commented Oct 5, 2023

Description

This PR really provides one major change to the enrich endpoint and then makes some minor adjustments to the link system to try and improve efficiency of the calls.

In general, this is pretty much as optimised as it can be - this does not make any headway to improving the self call, but there is very little we can do there as we don't know the scope of what is needed from the user structure - if it is a heavily modified user table (with a lot of relationships) we cannot improve the speed of this.

The biggest change here is that when the data provider requests a to enrich a row, it doesn't need all of the columns that come back, it generally only looks at a single field. In fact within the API functions of frontend-core it strips out everything else it gets back, so this is wasted if it is retrieved. I've added a field query string parameter to the enrich GET call which allows restricting the call down to exactly what the data provider needs, significantly improving the performance of this call.

Below is a before and after on a local network, note the difference in size as well, this makes a big difference in our cloud platform where the size creates extra latency ontop of the actual work done in the request.

Screenshots

Before (enrichment call)
image

After (enrichment call)
image

…ere is a problem with cyclic enrichment due to the outputProcessing, need to decide how to handle formulas on enrichment.
@mike12345567 mike12345567 self-assigned this Oct 5, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2023

Codecov Report

Merging #11981 (44c6401) into master (f1f706b) will increase coverage by 0.23%.
Report is 15 commits behind head on master.
The diff coverage is 76.52%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master   #11981      +/-   ##
==========================================
+ Coverage   74.38%   74.62%   +0.23%     
==========================================
  Files         326      326              
  Lines       13941    13925      -16     
  Branches     2937     2923      -14     
==========================================
+ Hits        10370    10391      +21     
+ Misses       3330     3299      -31     
+ Partials      241      235       -6     
Files Coverage Δ
packages/server/src/api/controllers/row/index.ts 97.81% <ø> (ø)
...ackages/server/src/api/controllers/row/internal.ts 86.99% <100.00%> (+1.83%) ⬆️
packages/server/src/api/controllers/row/utils.ts 78.12% <100.00%> (+48.95%) ⬆️
packages/server/src/api/controllers/user.ts 86.95% <100.00%> (-0.28%) ⬇️
packages/server/src/db/linkedRows/index.ts 96.63% <100.00%> (+0.37%) ⬆️
...ackages/server/src/utilities/rowProcessor/index.ts 69.65% <100.00%> (ø)
...es/server/src/api/controllers/row/staticFormula.ts 36.70% <0.00%> (ø)
packages/server/src/api/controllers/table/index.ts 69.14% <0.00%> (ø)
packages/server/src/sdk/app/rows/utils.ts 75.94% <75.00%> (-0.68%) ⬇️
packages/server/src/db/linkedRows/linkUtils.ts 84.37% <66.66%> (-3.77%) ⬇️
... and 2 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mike12345567 mike12345567 marked this pull request as ready for review October 12, 2023 10:09
@shogunpurple shogunpurple force-pushed the fix/internal-db-enrich-perf branch from 44c6401 to 5e00a0d Compare October 17, 2023 09:58
@shogunpurple shogunpurple deleted the fix/internal-db-enrich-perf branch October 17, 2023 12:38
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2023
@mike12345567 mike12345567 restored the fix/internal-db-enrich-perf branch October 17, 2023 13:09
@mike12345567 mike12345567 reopened this Oct 17, 2023
@mike12345567 mike12345567 added the firestorm Data/Infra/Revenue Team label Oct 18, 2023
Copy link
Member

@shogunpurple shogunpurple left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work

@mike12345567 mike12345567 merged commit 4ae2ae1 into master Oct 18, 2023
10 checks passed
@mike12345567 mike12345567 deleted the fix/internal-db-enrich-perf branch October 18, 2023 16:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants