Skip to content

Commit

Permalink
feat: Moved Tenant Information to Redis for quick access (#33309)
Browse files Browse the repository at this point in the history
## 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>
  • Loading branch information
NilanshBansal and mohanarpit authored May 15, 2024
1 parent b3ea6e2 commit 306e8c1
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
import org.springframework.data.annotation.Transient;
import org.springframework.data.mongodb.core.mapping.Document;

import java.io.Serializable;

@Getter
@Setter
@ToString
@NoArgsConstructor
@Document
@FieldNameConstants
public class Tenant extends BaseDomain {
public class Tenant extends BaseDomain implements Serializable {

@Unique String slug;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import lombok.Data;
import lombok.EqualsAndHashCode;

import java.io.Serializable;

@Data
@EqualsAndHashCode(callSuper = true)
public class TenantConfiguration extends TenantConfigurationCE {}
public class TenantConfiguration extends TenantConfigurationCE implements Serializable {}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
import lombok.Data;
import org.apache.commons.lang3.ObjectUtils;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

@Data
public class TenantConfigurationCE {
public class TenantConfigurationCE implements Serializable {

private String googleMapsKey;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.server.repositories.ce;

import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.User;
import reactor.core.publisher.Mono;

Expand All @@ -18,4 +19,8 @@ public interface CacheableRepositoryHelperCE {
Mono<String> getDefaultTenantId();

Mono<String> getInstanceAdminPermissionGroupId();

Mono<Tenant> fetchDefaultTenant(String tenantId);

Mono<Void> evictCachedTenant(String tenantId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.appsmith.server.domains.Config;
import com.appsmith.server.domains.PermissionGroup;
import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.TenantConfiguration;
import com.appsmith.server.domains.User;
import com.appsmith.server.domains.Workspace;
import com.appsmith.server.exceptions.AppsmithError;
Expand Down Expand Up @@ -166,4 +167,33 @@ public Mono<String> getInstanceAdminPermissionGroupId() {
.doOnSuccess(permissionGroupId ->
inMemoryCacheableRepositoryHelper.setInstanceAdminPermissionGroupId(permissionGroupId));
}

/**
* Returns the default tenant from the cache if present.
* If not present in cache, then it fetches the default tenant from the database and adds to redis.
* @param tenantId
* @return
*/
@Cache(cacheName = "tenant", key = "{#tenantId}")
@Override
public Mono<Tenant> fetchDefaultTenant(String tenantId) {
BridgeQuery<Tenant> defaultTenantCriteria = Bridge.equal(Tenant.Fields.slug, FieldName.DEFAULT);
BridgeQuery<Tenant> notDeletedCriteria = notDeleted();
BridgeQuery<Tenant> andCriteria = Bridge.and(defaultTenantCriteria, notDeletedCriteria);
Query query = new Query();
query.addCriteria(andCriteria);

return mongoOperations.findOne(query, Tenant.class).map(tenant -> {
if (tenant.getTenantConfiguration() == null) {
tenant.setTenantConfiguration(new TenantConfiguration());
}
return tenant;
});
}

@CacheEvict(cacheName = "tenant", key = "{#tenantId}")
@Override
public Mono<Void> evictCachedTenant(String tenantId) {
return Mono.empty().then();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import reactor.core.publisher.Mono;

public interface TenantRepositoryCE extends BaseRepository<Tenant, String>, CustomTenantRepositoryCE {

// Use tenantService.getDefaultTenant() instead of this method as it is cached to redis.
@Deprecated(forRemoval = true)
Mono<Tenant> findBySlug(String slug);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.appsmith.server.services;

import com.appsmith.server.helpers.FeatureFlagMigrationHelper;
import com.appsmith.server.repositories.CacheableRepositoryHelper;
import com.appsmith.server.repositories.TenantRepository;
import com.appsmith.server.services.ce.TenantServiceCEImpl;
import com.appsmith.server.solutions.EnvManager;
Expand All @@ -19,7 +20,15 @@ public TenantServiceImpl(
AnalyticsService analyticsService,
ConfigService configService,
@Lazy EnvManager envManager,
FeatureFlagMigrationHelper featureFlagMigrationHelper) {
super(validator, repository, analyticsService, configService, envManager, featureFlagMigrationHelper);
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper) {
super(
validator,
repository,
analyticsService,
configService,
envManager,
featureFlagMigrationHelper,
cacheableRepositoryHelper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
@Slf4j
@RequiredArgsConstructor
public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHelperCE {

private final TenantRepository tenantRepository;
private final ConfigService configService;
private final CloudServicesConfig cloudServicesConfig;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.helpers.FeatureFlagMigrationHelper;
import com.appsmith.server.repositories.CacheableRepositoryHelper;
import com.appsmith.server.repositories.TenantRepository;
import com.appsmith.server.services.AnalyticsService;
import com.appsmith.server.services.BaseService;
Expand Down Expand Up @@ -39,17 +40,21 @@ public class TenantServiceCEImpl extends BaseService<TenantRepository, Tenant, S

private final FeatureFlagMigrationHelper featureFlagMigrationHelper;

private final CacheableRepositoryHelper cacheableRepositoryHelper;

public TenantServiceCEImpl(
Validator validator,
TenantRepository repository,
AnalyticsService analyticsService,
ConfigService configService,
@Lazy EnvManager envManager,
FeatureFlagMigrationHelper featureFlagMigrationHelper) {
FeatureFlagMigrationHelper featureFlagMigrationHelper,
CacheableRepositoryHelper cacheableRepositoryHelper) {
super(validator, repository, analyticsService);
this.configService = configService;
this.envManager = envManager;
this.featureFlagMigrationHelper = featureFlagMigrationHelper;
this.cacheableRepositoryHelper = cacheableRepositoryHelper;
}

@Override
Expand All @@ -59,7 +64,6 @@ public Mono<String> getDefaultTenantId() {
if (StringUtils.hasLength(tenantId)) {
return Mono.just(tenantId);
}

return repository.findBySlug(FieldName.DEFAULT).map(Tenant::getId).map(tenantId -> {
// Set the cache value before returning.
this.tenantId = tenantId;
Expand All @@ -69,6 +73,7 @@ public Mono<String> getDefaultTenantId() {

@Override
public Mono<Tenant> updateTenantConfiguration(String tenantId, TenantConfiguration tenantConfiguration) {
Mono<Void> evictTenantCache = cacheableRepositoryHelper.evictCachedTenant(tenantId);
return repository
.findById(tenantId, MANAGE_TENANT)
.switchIfEmpty(Mono.error(
Expand All @@ -89,14 +94,23 @@ public Mono<Tenant> updateTenantConfiguration(String tenantId, TenantConfigurati
return Mono.empty();
});
}

return envMono.then(Mono.zip(Mono.just(oldtenantConfiguration), Mono.just(tenant)));
})
.flatMap(tuple2 -> {
Tenant tenant = tuple2.getT2();
TenantConfiguration oldConfig = tuple2.getT1();
AppsmithBeanUtils.copyNestedNonNullProperties(tenantConfiguration, oldConfig);
tenant.setTenantConfiguration(oldConfig);
return repository.updateById(tenantId, tenant, MANAGE_TENANT);
Mono<Tenant> updatedTenantMono = repository
.updateById(tenantId, tenant, MANAGE_TENANT)
.cache();
// Firstly updating the Tenant object in the database and then evicting the cache.
// returning the updatedTenant, notice the updatedTenantMono is cached using .cache()
// hence it will not be evaluated again
return updatedTenantMono
.then(Mono.defer(() -> evictTenantCache))
.then(updatedTenantMono);
});
}

Expand Down Expand Up @@ -151,16 +165,9 @@ public Mono<Tenant> getTenantConfiguration() {

@Override
public Mono<Tenant> getDefaultTenant() {
// Get the default tenant object from the DB and then populate the relevant user permissions in that
// We are doing this differently because `findBySlug` is a Mongo JPA query and not a custom Appsmith query
return repository
.findBySlug(FieldName.DEFAULT)
.map(tenant -> {
if (tenant.getTenantConfiguration() == null) {
tenant.setTenantConfiguration(new TenantConfiguration());
}
return tenant;
})
// Fetching Tenant from redis cache
return getDefaultTenantId()
.flatMap(tenantId -> cacheableRepositoryHelper.fetchDefaultTenant(tenantId))
.flatMap(tenant -> repository.setUserPermissionsInObject(tenant).switchIfEmpty(Mono.just(tenant)));
}

Expand Down Expand Up @@ -192,9 +199,12 @@ protected Mono<Tenant> getClientPertinentTenant(Tenant dbTenant, Tenant clientTe
return Mono.just(clientTenant);
}

// This function is used to save the tenant object in the database and evict the cache
@Override
public Mono<Tenant> save(Tenant tenant) {
return repository.save(tenant);
Mono<Void> evictTenantCache = cacheableRepositoryHelper.evictCachedTenant(tenantId);
Mono<Tenant> savedTenantMono = repository.save(tenant).cache();
return savedTenantMono.then(Mono.defer(() -> evictTenantCache)).then(savedTenantMono);
}

/**
Expand Down Expand Up @@ -242,6 +252,19 @@ public Mono<Tenant> retrieveById(String id) {
return repository.findById(id);
}

/**
* This function updates the tenant object in the database and evicts the cache
* @param tenantId
* @param tenant
* @return
*/
@Override
public Mono<Tenant> update(String tenantId, Tenant tenant) {
Mono<Void> evictTenantCache = cacheableRepositoryHelper.evictCachedTenant(tenantId);
Mono<Tenant> updatedTenantMono = super.update(tenantId, tenant).cache();
return updatedTenantMono.then(Mono.defer(() -> evictTenantCache)).then(updatedTenantMono);
}

/**
* This function checks if the tenant needs to be restarted and restarts after the feature flag migrations are
* executed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,14 @@ public void setup() throws IOException {

@AfterEach
public void cleanup() {
Tenant updatedTenant = new Tenant();
updatedTenant.setTenantConfiguration(originalTenantConfiguration);
tenantService
.getDefaultTenant()
.flatMap(tenant -> tenantRepository.updateAndReturn(
tenant.getId(),
Bridge.update().set(Tenant.Fields.tenantConfiguration, originalTenantConfiguration),
Optional.empty()))
.getDefaultTenantId()
.flatMap(tenantId -> tenantService.update(tenantId, updatedTenant))
.doOnError(error -> {
System.err.println("Error during cleanup: " + error.getMessage());
})
.block();
}

Expand Down

0 comments on commit 306e8c1

Please sign in to comment.