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

feat: Adding Tenant Information to Redis Cache for quick fetch #33641

Merged
merged 43 commits into from
May 28, 2024

Conversation

NilanshBansal
Copy link
Contributor

@NilanshBansal NilanshBansal commented May 22, 2024

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.

The old PR implementation #33309 had to be reverted due to the tenant GAC permissions not in sync between database and redis. RCA ref.
This PR uses the same branch as the old PR and builds the pending functionalities to complete the implementation.

Counterpart EE PR: https://github.com/appsmithorg/appsmith-ee/pull/4275

TL;DR
Adds tenant information tenantService.getDefaultTenant() to redis.

Fixes #33083, #33504, #33578

Automation

/ok-to-test tags="@tag.Settings"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9253953003
Commit: 7b4bf8d
Cypress dashboard url: Click here!

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

NilanshBansal and others added 21 commits May 6, 2024 13:51
…ervices/ce/TenantServiceCEImpl.java

Co-authored-by: Arpit Mohan <mohanarpit@users.noreply.github.com>
…g-to-redis' into feature/issue-33083/tenant-config-to-redis
@github-actions github-actions bot added Integrations Product Issues related to a specific integration Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Task A simple Todo Enhancement New feature or request labels May 22, 2024
@NilanshBansal NilanshBansal self-assigned this May 25, 2024
@NilanshBansal NilanshBansal requested review from sharat87 and abhvsn May 25, 2024 12:52
@NilanshBansal
Copy link
Contributor Author

@abhvsn @sharat87 can you please review this and this commits which are recently merged in. Other commits are older which originated from reopening the old merged PR.

These commits resolve the issues #33504 and #33578

@NilanshBansal NilanshBansal requested a review from trishaanand May 25, 2024 13:01
@NilanshBansal NilanshBansal requested review from a team and marks0351 and removed request for a team, pratapaprasanna, ankitakinger, ayushpahwa and marks0351 May 27, 2024 08:02
@NilanshBansal NilanshBansal changed the base branch from release to master May 27, 2024 08:13
@NilanshBansal NilanshBansal changed the base branch from master to release May 27, 2024 08:13
@NilanshBansal
Copy link
Contributor Author

All tests have passed earlier before this commit. Running only a subset of tests now as the change is only done to a server unit test file.
image

@NilanshBansal NilanshBansal added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels May 27, 2024
@NilanshBansal NilanshBansal added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels May 27, 2024
@NilanshBansal NilanshBansal merged commit b6b00d0 into release May 28, 2024
84 of 86 checks passed
@NilanshBansal NilanshBansal deleted the feature/issue-33083/tenant-config-to-redis branch May 28, 2024 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Product Issues related to a specific integration ok-to-test Required label for CI Task A simple Todo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Implement Redis caching for tenant configuration
2 participants