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

fix: Optimize job query with created_at and status indexes #267

Merged
merged 1 commit into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import type { JobCondition } from "@ctrlplane/validators/jobs";
import React from "react";
import { IconFilter, IconLoader2 } from "@tabler/icons-react";
import _ from "lodash";

import { Badge } from "@ctrlplane/ui/badge";
import { Button } from "@ctrlplane/ui/button";
Expand All @@ -28,6 +27,7 @@ import { VariableCell } from "~/app/[workspaceSlug]/(app)/_components/job-table/
import { JobTableStatusIcon } from "~/app/[workspaceSlug]/(app)/_components/JobTableStatusIcon";
import { useFilter } from "~/app/[workspaceSlug]/(app)/_components/useFilter";
import { api } from "~/trpc/react";
import { nFormatter } from "../../systems/[systemSlug]/_components/nFormatter";

type JobTableProps = {
workspaceId: string;
Expand All @@ -36,11 +36,17 @@ type JobTableProps = {
export const JobTable: React.FC<JobTableProps> = ({ workspaceId }) => {
const { filter, setFilter } = useFilter<JobCondition>();
const { setJobId } = useJobDrawer();
const allReleaseJobTriggers = api.job.config.byWorkspaceId.list.useQuery(
{ workspaceId, limit: 0 },
const { data: allJobsTotal } = api.job.config.byWorkspaceId.count.useQuery(
{ workspaceId },
{ refetchInterval: 60_000, placeholderData: (prev) => prev },
);

const { data: total, isLoading: isTotalLoading } =
api.job.config.byWorkspaceId.count.useQuery(
{ workspaceId, filter: filter ?? undefined },
{ refetchInterval: 60_000, placeholderData: (prev) => prev },
);

const releaseJobTriggers = api.job.config.byWorkspaceId.list.useQuery(
{ workspaceId, filter: filter ?? undefined, limit: 100 },
{ refetchInterval: 10_000, placeholderData: (prev) => prev },
Expand All @@ -67,22 +73,21 @@ export const JobTable: React.FC<JobTableProps> = ({ workspaceId }) => {
)}
</div>

{releaseJobTriggers.data?.total != null && (
<div className="flex items-center gap-2 rounded-lg border border-neutral-800/50 px-2 py-1 text-sm text-muted-foreground">
Total:
<Badge
variant="outline"
className="rounded-full border-neutral-800 text-inherit"
>
{releaseJobTriggers.data.total}
</Badge>
</div>
)}
<div className="flex items-center gap-2 rounded-lg border border-neutral-800/50 px-2 py-1 text-sm text-muted-foreground">
Total:
<Badge
variant="outline"
className="rounded-full border-neutral-800 text-inherit"
>
{isTotalLoading && <IconLoader2 className="h-3 w-3 animate-spin" />}
{!isTotalLoading && (total ? nFormatter(total, 1) : "-")}
</Badge>
</div>
</div>

{releaseJobTriggers.isLoading && (
<div className="space-y-2 p-4">
{_.range(10).map((i) => (
{Array.from({ length: 10 }).map((_, i) => (
<Skeleton
key={i}
className="h-9 w-full"
Expand All @@ -91,17 +96,17 @@ export const JobTable: React.FC<JobTableProps> = ({ workspaceId }) => {
))}
</div>
)}
{releaseJobTriggers.isSuccess && releaseJobTriggers.data.total === 0 && (
{releaseJobTriggers.isSuccess && total === 0 && (
<NoFilterMatch
numItems={allReleaseJobTriggers.data?.total ?? 0}
numItems={allJobsTotal ?? 0}
itemType="job"
onClear={() => setFilter(null)}
/>
)}

{releaseJobTriggers.isSuccess && releaseJobTriggers.data.total > 0 && (
<div className="h-[calc(100%-41px)] overflow-auto">
<Table className="table-fixed">
{releaseJobTriggers.isSuccess && releaseJobTriggers.data.length > 0 && (
<div className="h-[calc(100vh-93px)] overflow-auto">
<Table>
<TableHeader>
<TableRow>
<TableHead>Resource</TableHead>
Expand All @@ -114,13 +119,15 @@ export const JobTable: React.FC<JobTableProps> = ({ workspaceId }) => {
</TableRow>
</TableHeader>
<TableBody>
{releaseJobTriggers.data.items.map((job) => (
{releaseJobTriggers.data.map((job) => (
<TableRow
key={job.id}
onClick={() => setJobId(job.job.id)}
className="cursor-pointer"
>
<TableCell>{job.resource.name}</TableCell>
<TableCell>
<span className="flex truncate">{job.resource.name}</span>
</TableCell>
<TableCell>{job.environment.name}</TableCell>
<TableCell>{job.release.deployment.name}</TableCell>
<TableCell>{job.release.version}</TableCell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export default async function JobsPage({

const releaseJobTriggers = await api.job.config.byWorkspaceId.list({
workspaceId: workspace.id,
limit: 0,
limit: 1,
});

if (releaseJobTriggers.total === 0) return <JobsGettingStarted />;
if (releaseJobTriggers.length === 0) return <JobsGettingStarted />;

return <JobTable workspaceId={workspace.id} />;
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,13 @@ const ReleaseCell: React.FC<{
conditions: [isSameDeployment, isSameEnvironment, isSameRelease],
};

const { items } =
const releaseJobTriggers =
release != null
? await api.job.config.byWorkspaceId.list({
workspaceId: workspace.id,
filter,
})
: { items: null };
const releaseJobTriggers = items ?? [];
: [];
const hasResources = env.resources.length > 0;
const hasRelease = release != null;
const jc = releaseJobTriggers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ const WaitingOnActiveCheck: React.FC<ReleaseSequencingNodeProps["data"]> = ({
const loading = pendingJobsQ.isLoading || inProgressJobsQ.isLoading;

const isCurrentReleasePending =
pendingJobsQ.data != null && pendingJobsQ.data.total > 0;
pendingJobsQ.data != null && pendingJobsQ.data.length > 0;
const isSeparateReleaseInProgress =
inProgressJobsQ.data != null && inProgressJobsQ.data.total > 0;
inProgressJobsQ.data != null && inProgressJobsQ.data.length > 0;
Comment on lines +135 to +137
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Inconsistent array length checks found in job and release queries

The codebase shows mixed usage of .total and .length for checking array sizes. This inconsistency should be addressed as part of the job query optimization efforts.

Key locations requiring attention:

  • useReleaseChannel.ts: Uses data?.total
  • DeploymentPageContent.tsx: Uses data?.total
  • ReleaseSequencingNode.tsx: Contains both patterns - the updated .length check for jobs and .total for releases
  • JobHistoryChart.tsx: Uses data?.total
  • SearchDialog.tsx: Uses data?.total
  • AppSidebarPopoverResources.tsx: Uses both data?.total and data?.items.length
  • ResourcePageContent.tsx: Uses data?.total
🔗 Analysis chain

LGTM! Consistent array length checks.

The change from .total to .length aligns with the job query optimization efforts. The null checks are properly maintained.

Let's verify that this pattern is consistently applied across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining uses of .total in job-related queries
rg -A 2 'data\?\.total' --type ts

Length of output: 4174


const isWaitingOnActive =
isCurrentReleasePending && isSeparateReleaseInProgress;
Expand Down
33 changes: 22 additions & 11 deletions packages/api/src/router/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { z } from "zod";

import {
and,
asc,
countDistinct,
desc,
eq,
Expand Down Expand Up @@ -137,8 +136,8 @@ const releaseJobTriggerRouter = createTRPCRouter({
.perform(Permission.JobList)
.on({ type: "workspace", id: input.workspaceId }),
})
.query(async ({ ctx, input }) => {
const items = await releaseJobTriggerQuery(ctx.db)
.query(({ ctx, input }) =>
releaseJobTriggerQuery(ctx.db)
.leftJoin(
schema.system,
eq(schema.system.id, schema.deployment.systemId),
Expand All @@ -158,7 +157,7 @@ const releaseJobTriggerRouter = createTRPCRouter({
schema.releaseJobMatchesCondition(ctx.db, input.filter),
),
)
.orderBy(asc(schema.releaseJobTrigger.createdAt))
.orderBy(desc(schema.job.createdAt))
.limit(input.limit)
.offset(input.offset)
.then((data) =>
Expand All @@ -185,9 +184,23 @@ const releaseJobTriggerRouter = createTRPCRouter({
environment: v[0]!.environment,
}))
.value(),
);

const total = await ctx.db
),
),
count: protectedProcedure
.input(
z.object({
workspaceId: z.string().uuid(),
filter: jobCondition.optional(),
}),
)
.meta({
authorizationCheck: ({ canUser, input }) =>
canUser
.perform(Permission.JobList)
.on({ type: "workspace", id: input.workspaceId }),
})
.query(({ ctx, input }) =>
ctx.db
.select({
count: countDistinct(schema.releaseJobTrigger.id),
})
Expand Down Expand Up @@ -224,10 +237,8 @@ const releaseJobTriggerRouter = createTRPCRouter({
),
)
.then(takeFirst)
.then((t) => t.count);

return { items, total };
}),
.then((t) => t.count),
),
dailyCount: protectedProcedure
.input(z.object({ workspaceId: z.string().uuid(), timezone: z.string() }))
.meta({
Expand Down
2 changes: 2 additions & 0 deletions packages/db/drizzle/0046_little_warstar.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CREATE INDEX IF NOT EXISTS "job_created_at_idx" ON "job" USING btree ("created_at");--> statement-breakpoint
CREATE INDEX IF NOT EXISTS "job_status_idx" ON "job" USING btree ("status");
Loading
Loading