From c21381ba041e2ba1a025209606457d655af272dc Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Thu, 20 Mar 2025 10:09:26 +0530 Subject: [PATCH] chore: revert optimisations related execute flow (#39800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Reverted** the following optimisations: - Pushed out the sendExecuteAnalyticsEvent from the critical path of returning action's exection result - Improved the critical Path sendExecuteAnalyticsEvent by running the application mono concurrent to other events ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!CAUTION] > 🔴 🔴 🔴 Some tests have failed. > Workflow run: > Commit: fda9eafe6d03be4021f9f7a83aaf6d1c92fb7490 > Cypress dashboard. > Tags: @tag.All > Spec: > The following are new failures, please fix them before merging the PR:
    >
  1. cypress/e2e/Regression/ClientSide/BugTests/DS_Bug26941_Spec.ts
> List of identified flaky tests. >
Wed, 19 Mar 2025 15:43:49 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Refactor** - Streamlined the processing of analytics events to enhance overall system performance and maintainability, ensuring a more stable and responsive experience for end users. --- .../ce/ActionExecutionSolutionCEImpl.java | 33 +++++++------------ 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java index 7f0dc2ecd3..5dd924133b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ActionExecutionSolutionCEImpl.java @@ -68,8 +68,6 @@ import org.springframework.util.StringUtils; import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -import reactor.core.scheduler.Schedulers; -import reactor.util.function.Tuple2; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -925,33 +923,26 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE .onErrorMap(executionExceptionMapper(actionDTO, timeoutDuration)) .onErrorResume(executionExceptionHandler(actionDTO)) .elapsed() - .map(tuple1 -> { + .flatMap(tuple1 -> { Long timeElapsed = tuple1.getT1(); + ActionExecutionResult result = tuple1.getT2(); + log.debug( "{}: Action {} with id {} execution time : {} ms", Thread.currentThread().getName(), actionDTO.getName(), actionDTO.getId(), timeElapsed); - return tuple1; - }) - .doOnSuccess(tuple2 -> { - Long timeElapsed = tuple2.getT1(); - ActionExecutionResult result = tuple2.getT2(); - // Runs the analytics in the separate thread and immediately return the execution result - sendExecuteAnalyticsEvent( + + return sendExecuteAnalyticsEvent( actionDTO, datasourceStorage, executeActionDTO, result, timeElapsed, finalRawActionConfiguration) - .name(SEND_EXECUTE_ANALYTICS_EVENT) - .tap(Micrometer.observation(observationRegistry)) - .subscribeOn(Schedulers.boundedElastic()) - .subscribe(); - }) - .map(Tuple2::getT2); + .thenReturn(result); + }); }); } @@ -1109,16 +1100,16 @@ public class ActionExecutionSolutionCEImpl implements ActionExecutionSolutionCE request.setProperties(stringProperties); } - Mono applicationMono = Mono.justOrEmpty(actionDTO.getApplicationId()) + return Mono.justOrEmpty(actionDTO.getApplicationId()) .flatMap(applicationService::findById) - .defaultIfEmpty(new Application()); - return Mono.zip( - applicationMono, + .defaultIfEmpty(new Application()) + .flatMap(application -> Mono.zip( + Mono.just(application), sessionUserService.getCurrentUser(), newPageService.getNameByPageId(actionDTO.getPageId(), executeActionDto.getViewMode()), pluginService.getByIdWithoutPermissionCheck(actionDTO.getPluginId()), datasourceStorageService.getEnvironmentNameFromEnvironmentIdForAnalytics( - datasourceStorage.getEnvironmentId())) + datasourceStorage.getEnvironmentId()))) .flatMap(tuple -> { final Application application = tuple.getT1(); final User user = tuple.getT2();