perf: /router endpoint perf improvement #18366

hariombalhara · 2024-12-24T09:46:41Z

What does this PR do?

Using possibly faster findUnique for form query instead of findFirst
Abstracted out handleResponse from response.handler.ts to be reused in /router - It allows already available form to be passed directly to handleResponse and avoids one sequential query to DB.
Passing orgId directly to findTeamMembersMatchingAttributeLogic which helps in restricting the number of records being queried in attributes related tables.
Completely different querying approach for getAttributesAssignmentData avoiding some queries in Prisma and using some in memory calculations.
Moved /router and trpc /response endpoint out of app-store as it was not allowing to use the repositories. Prisma was getting imported client side - Note: Moving /router to app-router can be done later.

Improvement seen locally:

With 2000 members in Acme and half of them being part of the Insights team, querying time went from 442ms(minimum) to 271ms(minimum). The improvements should be more visible with production data which has much much higher number of records.

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Test Preview of Routing Form
/router endpoint should also work
A regular routing form submission should also work

Followup

A teamMembershipId column in attributeToUser could possibly save some querying time, if in-memory computation for orgMembershipId to userId doesn't work

github-actions · 2024-12-24T09:46:56Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Add stats and allow showing embed without loader". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

vercel · 2024-12-24T09:46:56Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview		Dec 25, 2024 1:10pm
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview		Dec 25, 2024 1:10pm

hariombalhara · 2024-12-24T10:35:50Z

packages/app-store/routing-forms/pages/router/getServerSideProps.ts

@@ -178,6 +186,9 @@ export const getServerSideProps = async function getServerSideProps(
      serializableForm.fields
    );

+    // TODO: To be done using sentry tracing
+    console.log("Server-Timing", getServerTimingHeader(timeTaken));


timeTaken is already being computed, so I will just use the same approach for now for quick release. I will later move it to Sentry.

hariombalhara · 2024-12-24T10:37:24Z

packages/app-store/routing-forms/pages/router/getServerSideProps.ts

-  const { createContext } = await import("@calcom/trpc/server/createContext");
-  const ctx = await createContext(context);
-
-  const { default: trpcRouter } = await import("@calcom/app-store/routing-forms/trpc/_router");
-  const caller = trpcRouter.createCaller(ctx);


Avoid using another trpcHandler and instead use the utility directly.

Also, needed to do this so that I can easily pass on already queried form to handleResponse and avoid repeat querying of same data

hariombalhara · 2024-12-24T11:09:27Z

packages/app-store/routing-forms/trpc/response.handler.ts

+    include: {
+      team: {
+        select: {
+          parentId: true,


New requirement, it was already queried in /router endpoint for some other requirement

hariombalhara · 2024-12-24T11:37:13Z

packages/app-store/routing-forms/pages/router/getServerSideProps.ts

-
-  const form = await prisma.app_RoutingForms_Form.findFirst({
+async function findFormById(formId: string, prisma: AppPrisma) {
+  return await prisma.app_RoutingForms_Form.findUnique({


Using findUnique instead of findFirst

github-actions · 2024-12-24T14:54:45Z

E2E results are ready!

Workflow #42361.1 latest results

hariombalhara · 2024-12-25T10:27:36Z

apps/web/next.config.js

-        source: "/router/:path*",
-        destination: "/apps/routing-forms/router/:path*",
-      },


Rewrite not needed because /router is actually a route now

hariombalhara · 2024-12-25T10:31:16Z

apps/web/server/lib/router/getServerSideProps.ts

-    slug: z.string(),
-    pages: z.array(z.string()),
  })


Slug pages was there because it was earlier under app-store. Now it is not needed.

hariombalhara · 2024-12-25T12:53:19Z

apps/web/server/lib/router/getServerSideProps.ts

+  const { form: formId, embed, ...fieldsResponses } = queryParsed.data;
+  const { currentOrgDomain } = orgDomainConfig(context.req);
+
+  let timeTaken: Record<string, number | null> = {};


Commented at other places as well but comment here as well. Time Taken is an existing approach that I am adding more data on. This later needs to move to Sentry performance tracing

hariombalhara · 2024-12-25T12:54:11Z

apps/web/server/lib/router/getServerSideProps.ts

-    const result = await caller.public.response({
-      formId: form.id,
+    const result = await handleResponse({
+      form: serializableForm,


Passing already queried form now.

hariombalhara · 2024-12-25T12:56:33Z

packages/app-store/routing-forms/lib/handleResponse.ts

-        : null;
+
+      const teamMembersMatchingAttributeLogicWithResult =
+        formTeamId && formOrgId


Team Members matching logic is applicable only for organization

hariombalhara · 2024-12-25T12:58:46Z

packages/app-store/routing-forms/pages/app-routing.config.tsx

-import * as Router from "./router/[...appPages]";
-import { getServerSideProps as getServerSidePropsRouter } from "./router/getServerSideProps";


Moved outside of app-pages

hariombalhara · 2024-12-25T12:59:17Z

packages/app-store/routing-forms/pages/routing-link/getServerSideProps.ts

@@ -6,7 +6,7 @@ import type { AppGetServerSidePropsContext, AppPrisma } from "@calcom/types/AppG
 import { enrichFormWithMigrationData } from "../../enrichFormWithMigrationData";
 import { getSerializableForm } from "../../lib/getSerializableForm";

-export async function isAuthorizedToViewTheForm({
+export function isAuthorizedToViewTheForm({


It was async accidentally

hariombalhara force-pushed the routing-perf branch from acba639 to 9379810 Compare December 24, 2024 09:46

keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Dec 24, 2024

hariombalhara changed the title ~~Add stats and allow showing embed without loader~~ perf: /router endpoint perf improvement Dec 24, 2024

vercel bot had a problem deploying to Preview – api December 24, 2024 09:51 Failure

hariombalhara commented Dec 24, 2024

View reviewed changes

hariombalhara force-pushed the routing-perf branch from 9379810 to 86bb2ba Compare December 24, 2024 11:06

github-actions bot added the ❗️ migrations contains migration files label Dec 24, 2024

hariombalhara commented Dec 24, 2024

View reviewed changes

vercel bot deployed to Preview – api December 24, 2024 11:14 View deployment

hariombalhara force-pushed the routing-perf branch from 86bb2ba to 1d9d8cb Compare December 24, 2024 11:36

hariombalhara commented Dec 24, 2024

View reviewed changes

vercel bot deployed to Preview – api December 24, 2024 11:43 View deployment

hariombalhara force-pushed the routing-perf branch from 1d9d8cb to 6f56f8e Compare December 24, 2024 11:55

vercel bot deployed to Preview – api December 24, 2024 12:02 View deployment

hariombalhara added the ready-for-e2e label Dec 24, 2024

Improve /router performance

3dff0ca

hariombalhara force-pushed the routing-perf branch from 6f56f8e to 3dff0ca Compare December 24, 2024 12:20

vercel bot deployed to Preview – api December 24, 2024 12:27 View deployment

Fix prisma import error onm client

a6aebf4

vercel bot deployed to Preview – api December 24, 2024 14:41 View deployment

hariombalhara commented Dec 25, 2024

View reviewed changes

SelfReview feedback

f91b1cf

vercel bot deployed to Preview – api December 25, 2024 13:17 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: /router endpoint perf improvement #18366

perf: /router endpoint perf improvement #18366

hariombalhara commented Dec 24, 2024 •

edited

Loading

github-actions bot commented Dec 24, 2024 •

edited

Loading

vercel bot commented Dec 24, 2024 •

edited

Loading

hariombalhara Dec 24, 2024

hariombalhara Dec 24, 2024

hariombalhara Dec 24, 2024 •

edited

Loading

hariombalhara Dec 24, 2024

github-actions bot commented Dec 24, 2024 •

edited

Loading

hariombalhara Dec 25, 2024

hariombalhara Dec 25, 2024

hariombalhara Dec 25, 2024

hariombalhara Dec 25, 2024

hariombalhara Dec 25, 2024

hariombalhara Dec 25, 2024

hariombalhara Dec 25, 2024

		import * as Router from "./router/[...appPages]";
		import { getServerSideProps as getServerSidePropsRouter } from "./router/getServerSideProps";

perf: /router endpoint perf improvement #18366

Are you sure you want to change the base?

perf: /router endpoint perf improvement #18366

Conversation

hariombalhara commented Dec 24, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

Followup

github-actions bot commented Dec 24, 2024 • edited Loading

vercel bot commented Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hariombalhara Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 24, 2024 • edited Loading

E2E results are ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hariombalhara commented Dec 24, 2024 •

edited

Loading

github-actions bot commented Dec 24, 2024 •

edited

Loading

vercel bot commented Dec 24, 2024 •

edited

Loading

hariombalhara Dec 24, 2024 •

edited

Loading

github-actions bot commented Dec 24, 2024 •

edited

Loading