Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

chore: improving /actions endpoint performance (#22109)

Browse Source 
## Description

/actions endpoint has very high response latency, The leading factor for
this latency is the DB calls to collections which have high volume of
data. i.e `actions` and `applications`. This Pr tries to address this by
optimising the flow to reduce db calls wherever possible.

The following strategies have been used to reduce the latency as much as
possible

- Reduced DB calls for fetching application and related permission
- Added Criteria for removing JS function from the repository call
- Added a in memory cache to hold default plugins. (this would be used
for filling missing details in action from plugin) resulted in 25-28%
performance advantage.
- **A good next step would have been adding index on all query
parameters, however have held on to that for some data.**

In the PR we have added Micrometer observability for /actions endpoint
as for more data.

> This PR addresses tries to improve the performance of the above
endpoints by means of reducing redundant calls to DB, and caching
wherever possible.

Fixes #21850 

## Type of change
- Chore (housekeeping or task changes that don't impact user perception)

## This has been tested:
- Manual

## Progress: 
The DP created with this PR was tested, however the results were not
consistent. waiting for testing dashboard to be ready to verify the
median/p95 timings from the data

## Checklist:
### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag
This commit is contained in:
Manish Kumar 2023-05-05 14:01:24 +05:30 committed by GitHub
parent 9a42aff4c3
commit 0b6a0f30af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 207 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ActionSpans.java vendored
View File

@ -13,5 +13,9 @@ public final class ActionSpans {
public static final String ACTION_EXECUTION_VALIDATE_AUTHENTICATION = "validate.authentication";
public static final String ACTION_EXECUTION_PLUGIN_EXECUTION = "total.plugin.execution";
public static final String ACTION_EXECUTION_SERVER_EXECUTION = "total.server.execution";
public static final String GET_UNPUBLISHED_ACTION = "get.action.unpublished";
public static final String GET_VIEW_MODE_ACTION = "get.action.viewmode";
public static final String GET_ACTION_REPOSITORY_CALL = "get.action.repository.call";
}

11
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java
View File

@ -36,8 +36,9 @@ public interface CustomNewActionRepositoryCE extends AppsmithRepository<NewActio
Flux<NewAction> findAllActionsByNameAndPageIdsAndViewMode(String name, List<String> pageIds, Boolean viewMode, AclPermission aclPermission, Sort sort);
Flux<NewAction> findUnpublishedActionsByNameInAndPageIdAndExecuteOnLoadTrue(
Set<String> names, String pageId, AclPermission permission);
Flux<NewAction> findUnpublishedActionsByNameInAndPageIdAndExecuteOnLoadTrue(Set<String> names,
String pageId,
AclPermission permission);
Flux<NewAction> findByApplicationId(String applicationId, AclPermission aclPermission, Sort sort);
@ -56,4 +57,10 @@ public interface CustomNewActionRepositoryCE extends AppsmithRepository<NewActio
Flux<NewAction> findByListOfPageIds(List<String> pageIds, AclPermission permission);
Flux<NewAction> findByListOfPageIds(List<String> pageIds, Optional<AclPermission> permission);
Flux<NewAction> findNonJsActionsByApplicationIdAndViewMode(String applicationId, Boolean viewMode, AclPermission aclPermission);
Flux<NewAction> findAllNonJsActionsByNameAndPageIdsAndViewMode(String name, List<String> pageIds,
Boolean viewMode,
AclPermission aclPermission,
Sort sort);
}

67
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java
View File

@ -1,5 +1,6 @@
package com.appsmith.server.repositories.ce;
import com.appsmith.external.models.PluginType;
import com.appsmith.external.models.QActionConfiguration;
import com.appsmith.external.models.QBranchAwareDomain;
import com.appsmith.server.acl.AclPermission;
@ -319,7 +320,7 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
Criteria gitSyncIdCriteria = where(FieldName.GIT_SYNC_ID).is(gitSyncId);
return queryFirst(List.of(defaultAppIdCriteria, gitSyncIdCriteria), permission);
}
@Override
public Flux<NewAction> findByListOfPageIds(List<String> pageIds, AclPermission permission) {
@ -336,4 +337,68 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
return queryAll(List.of(pageIdCriteria), permission);
}
@Override
public Flux<NewAction> findNonJsActionsByApplicationIdAndViewMode(String applicationId, Boolean viewMode,
AclPermission aclPermission) {
List<Criteria> criteria = new ArrayList<>();
Criteria applicationCriterion = where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
criteria.add(applicationCriterion);
Criteria nonJsTypeCriteria = where(fieldName(QNewAction.newAction.pluginType)).ne(PluginType.JS);
criteria.add(nonJsTypeCriteria);
if (Boolean.FALSE.equals(viewMode)) {
// In case an action has been deleted in edit mode, but still exists in deployed mode, NewAction object would exist. To handle this, only fetch non-deleted actions
Criteria deletedCriterion = where(fieldName(QNewAction.newAction.unpublishedAction) + "." + fieldName(QNewAction.newAction.unpublishedAction.deletedAt)).is(null);
criteria.add(deletedCriterion);
}
return queryAll(criteria, aclPermission);
}
@Override
public Flux<NewAction> findAllNonJsActionsByNameAndPageIdsAndViewMode(String name, List<String> pageIds,
Boolean viewMode, AclPermission aclPermission,
Sort sort) {
List<Criteria> criteriaList = new ArrayList<>();
Criteria nonJsTypeCriteria = where(fieldName(QNewAction.newAction.pluginType)).ne(PluginType.JS);
criteriaList.add(nonJsTypeCriteria);
// Fetch published actions
if (Boolean.TRUE.equals(viewMode)) {
if (name != null) {
Criteria nameCriteria = where(fieldName(QNewAction.newAction.publishedAction) + "." + fieldName(QNewAction.newAction.publishedAction.name)).is(name);
criteriaList.add(nameCriteria);
}
if (pageIds != null && !pageIds.isEmpty()) {
Criteria pageCriteria = where(fieldName(QNewAction.newAction.publishedAction) + "." + fieldName(QNewAction.newAction.publishedAction.pageId)).in(pageIds);
criteriaList.add(pageCriteria);
}
}
// Fetch unpublished actions
else {
if (name != null) {
Criteria nameCriteria = where(fieldName(QNewAction.newAction.unpublishedAction) + "." + fieldName(QNewAction.newAction.unpublishedAction.name)).is(name);
criteriaList.add(nameCriteria);
}
if (pageIds != null && !pageIds.isEmpty()) {
Criteria pageCriteria = where(fieldName(QNewAction.newAction.unpublishedAction) + "." + fieldName(QNewAction.newAction.unpublishedAction.pageId)).in(pageIds);
criteriaList.add(pageCriteria);
}
// In case an action has been deleted in edit mode, but still exists in deployed mode, NewAction object would exist. To handle this, only fetch non-deleted actions
Criteria deletedCriteria = where(fieldName(QNewAction.newAction.unpublishedAction) + "." + fieldName(QNewAction.newAction.unpublishedAction.deletedAt)).is(null);
criteriaList.add(deletedCriteria);
}
return queryAll(criteriaList, aclPermission, sort);
}
}

4
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCE.java
View File

@ -73,6 +73,10 @@ public interface NewActionServiceCE extends CrudService<NewAction, String> {
Mono<ActionDTO> deleteUnpublishedAction(String id);
Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, Boolean includeJsActions);
Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, String branchName, Boolean includeJsActions);
Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params);
Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, String branchName);

140
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java
View File

@ -127,6 +127,9 @@ import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION
import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_REQUEST_PARSING;
import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_SERVER_EXECUTION;
import static com.appsmith.external.constants.spans.ActionSpans.ACTION_EXECUTION_VALIDATE_AUTHENTICATION;
import static com.appsmith.external.constants.spans.ActionSpans.GET_ACTION_REPOSITORY_CALL;
import static com.appsmith.external.constants.spans.ActionSpans.GET_UNPUBLISHED_ACTION;
import static com.appsmith.external.constants.spans.ActionSpans.GET_VIEW_MODE_ACTION;
import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNewFieldValuesIntoOldObject;
import static com.appsmith.external.helpers.DataTypeStringUtils.getDisplayDataTypes;
import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData;
@ -178,6 +181,8 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
private final ActionPermission actionPermission;
private final ObservationRegistry observationRegistry;
private final Map<String, Plugin> defaultPluginMap = new HashMap<>();
private final AtomicReference<Plugin> jsTypePluginReference = new AtomicReference<>();
public NewActionServiceCEImpl(Scheduler scheduler,
Validator validator,
@ -1513,14 +1518,17 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
return Mono.just(action);
})
.flatMap(this::sanitizeAction);
.collectList()
.flatMapMany(this::addMissingPluginDetailsIntoAllActions);
}
@Override
public Flux<ActionViewDTO> getActionsForViewMode(String defaultApplicationId, String branchName) {
return applicationService.findBranchedApplicationId(branchName, defaultApplicationId, applicationPermission.getReadPermission())
.flatMapMany(this::getActionsForViewMode)
.map(responseUtils::updateActionViewDTOWithDefaultResources);
.map(responseUtils::updateActionViewDTOWithDefaultResources)
.name(GET_VIEW_MODE_ACTION)
.tap(Micrometer.observation(observationRegistry));
}
@Override
@ -1711,7 +1719,7 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
}
@Override
public Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params) {
public Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, Boolean includeJsActions) {
String name = null;
List<String> pageIds = new ArrayList<>();
@ -1726,22 +1734,50 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
pageIds.add(params.getFirst(FieldName.PAGE_ID));
}
Flux<NewAction> actionsFromRepository;
if (params.getFirst(FieldName.APPLICATION_ID) != null) {
// Fetch unpublished pages because GET actions is only called during edit mode. For view mode, different
// function call is made which takes care of returning only the essential fields of an action
return applicationService
.findById(params.getFirst(FieldName.APPLICATION_ID), applicationPermission.getReadPermission())
.flatMapMany(application -> repository.findByApplicationIdAndViewMode(application.getId(), false, actionPermission.getReadPermission()))
.flatMap(this::sanitizeAction)
.flatMap(this::setTransientFieldsInUnpublishedAction);
if (FALSE.equals(includeJsActions)) {
actionsFromRepository = repository.findNonJsActionsByApplicationIdAndViewMode(params.getFirst(FieldName.APPLICATION_ID),
false,
actionPermission.getReadPermission());
} else {
actionsFromRepository = repository.findByApplicationIdAndViewMode(params.getFirst(FieldName.APPLICATION_ID),
false,
actionPermission.getReadPermission());
}
} else {
if (FALSE.equals(includeJsActions)) {
actionsFromRepository = repository.findAllNonJsActionsByNameAndPageIdsAndViewMode(name, pageIds, false,
actionPermission.getReadPermission(),
sort);
} else {
actionsFromRepository = repository.findAllActionsByNameAndPageIdsAndViewMode(name, pageIds, false,
actionPermission.getReadPermission(),
sort);
}
}
return repository.findAllActionsByNameAndPageIdsAndViewMode(name, pageIds, false, actionPermission.getReadPermission(), sort)
.flatMap(this::sanitizeAction)
.flatMap(this::setTransientFieldsInUnpublishedAction);
return actionsFromRepository
.collectList()
.flatMapMany(this::addMissingPluginDetailsIntoAllActions)
.flatMap(this::setTransientFieldsInUnpublishedAction)
// this generates four different tags, (ApplicationId, FieldId) *(True, False)
.tag("includeJsAction", (params.get(FieldName.APPLICATION_ID) == null ? FieldName.PAGE_ID: FieldName.APPLICATION_ID )+ includeJsActions.toString())
.name(GET_ACTION_REPOSITORY_CALL)
.tap(Micrometer.observation(observationRegistry));
}
@Override
public Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, String branchName) {
public Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params,
String branchName,
Boolean includeJsActions) {
MultiValueMap<String, String> updatedParams = new LinkedMultiValueMap<>(params);
// Get branched applicationId and pageId
@ -1762,21 +1798,33 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
if (!CollectionUtils.isEmpty(params.get(FieldName.APPLICATION_ID)) && !StringUtils.isEmpty(applicationId)) {
updatedParams.set(FieldName.APPLICATION_ID, applicationId);
}
return getUnpublishedActions(updatedParams);
return getUnpublishedActions(updatedParams, includeJsActions);
})
.map(responseUtils::updateActionDTOWithDefaultResources);
}
@Override
public Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params) {
return getUnpublishedActions(params, TRUE);
}
@Override
public Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, String branchName) {
return getUnpublishedActions(params, branchName, TRUE);
}
@Override
public Flux<ActionDTO> getUnpublishedActionsExceptJs(MultiValueMap<String, String> params) {
return this.getUnpublishedActions(params)
return this.getUnpublishedActions(params, FALSE)
.filter(actionDTO -> !PluginType.JS.equals(actionDTO.getPluginType()));
}
@Override
public Flux<ActionDTO> getUnpublishedActionsExceptJs(MultiValueMap<String, String> params, String branchName) {
return this.getUnpublishedActions(params, branchName)
.filter(actionDTO -> !PluginType.JS.equals(actionDTO.getPluginType()));
return this.getUnpublishedActions(params, branchName, FALSE)
.filter(actionDTO -> !PluginType.JS.equals(actionDTO.getPluginType()))
.name(GET_UNPUBLISHED_ACTION)
.tap(Micrometer.observation(observationRegistry));
}
/**
@ -1797,6 +1845,66 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
return actionMono;
}
public Flux<NewAction> addMissingPluginDetailsIntoAllActions(List<NewAction> actionList) {
Mono<Map<String, Plugin>> pluginMapMono = Mono.just(defaultPluginMap);
/* This conditional would be false once per pod per restart, as soon as first request goes through
the default plugin map will have all the plugins and subsequent requests might not require to fetch again.
*/
if (CollectionUtils.isEmpty(defaultPluginMap)) {
pluginMapMono = pluginService.getDefaultPlugins()
.collectMap(Plugin::getId)
.map(pluginMap -> {
pluginMap.forEach((pluginId, plugin) -> {
defaultPluginMap.put(pluginId, plugin);
if (JS_PLUGIN_PACKAGE_NAME
.equals(plugin.getPackageName())) {
jsTypePluginReference.set(plugin);
}
});
return pluginMap;
});
}
return pluginMapMono
.thenMany(Flux.fromIterable(actionList))
.flatMap(action-> {
if (!isPluginTypeOrPluginIdMissing(action)) {
return Mono.just(action);
}
return addMissingPluginDetailsToNewActionObjects(action);
});
}
private Mono<NewAction> addMissingPluginDetailsToNewActionObjects(NewAction action) {
ActionDTO actionDTO = action.getUnpublishedAction();
if (actionDTO == null) {
return Mono.just(action);
}
Datasource datasource = actionDTO.getDatasource();
if (actionDTO.getCollectionId() != null) {
action.setPluginType(JS_PLUGIN_TYPE);
action.setPluginId(jsTypePluginReference.get().getId());
return Mono.just(action);
} else if (datasource != null && datasource.getPluginId() != null) {
String pluginId = datasource.getPluginId();
action.setPluginId(pluginId);
if (defaultPluginMap.containsKey(pluginId)) {
Plugin plugin = defaultPluginMap.get(pluginId);
action.setPluginType(plugin.getType());
} else {
setPluginTypeFromId(action, pluginId);
}
}
return Mono.just(action);
}
@Override
public Mono<ActionDTO> fillSelfReferencingDataPaths(ActionDTO actionDTO) {
Mono<Plugin> pluginMono = pluginService.getById(actionDTO.getPluginId());