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
[Task]: Implement Redis caching for tenant configuration #33083
Comments
Picking this up after the release blocker issue gets resolved |
## Description The tenant is fetched multiple times across the appsmith codebase but is rarely updated (from the admin settings). Every time a fetch call to the database is costly both in terms of resources and time taken. The consolidated api also makes a call to fetch the tenant and return to the client. To improve the performance of fetching the tenant information, we are moving the tenant information to redis cache for quicker fetch. This will improve the performance of the consolidated api and also reduce the time taken by all the different functionalities within the backend codebase which depend on tenant to process further. **TL;DR** Adds tenant information `tenantService.getDefaultTenant()` to redis. Fixes #33083 ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9079438120> > Commit: 306e77f > Cypress dashboard url: <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9079438120&attempt=2" target="_blank">Click here!</a> <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No --------- Co-authored-by: Arpit Mohan <mohanarpit@users.noreply.github.com>
Reopening as the task had to be reverted. |
Updated estimates to 3 as per our discussion in sprint planning meeting. |
Description
The tenants/current api is the slowest component of consolidated/view api. Reducing the latency of tenants/current api will reduce the latency of backend in view experience for all users.
Relevant Slack thread : https://theappsmith.slack.com/archives/C024GUDM0LT/p1713933688987279
In the diagram in the slack thread, the blue line just below the magenta line at the top is the contribution of tenants/current to consolidated api. The diagram shows that tenants/current is 90% contributor to consolidated view api.
Upon debugging the cause of latency from tenants/current, it is found that the api makes a DB call to fetch the tenant configuration every time.
Caching the tenant config ID is already present in the codebase. Extending that approach, we can cache the whole tenant config and update it when a user changes admin settings.
Not making the DB call will reduce the latency of tenants/current api and decrease the latency of consolidated view api as well.
CE PR: #33641
Counterpart EE PR: https://github.com/appsmithorg/appsmith-ee/pull/4275
Subtasks
The text was updated successfully, but these errors were encountered: