mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-30 19:32:46 +08:00
perf(db): logs-list index, drop redundant indexes, replica routing, hot-path write cleanups (#5105)
* perf(db): logs-list index, drop redundant indexes, replica routing, hot-path write cleanups * fix(logs): keep /api/v1/logs on primary db — its permissions join is the auth gate, not replica-safe
This commit is contained in:
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { copilotHttpMock, copilotHttpMockFns } from '@sim/testing'
|
||||
import { NextRequest } from 'next/server'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mockUpdate, mockSet, mockWhere, mockParseRequest } = vi.hoisted(() => ({
|
||||
mockUpdate: vi.fn(),
|
||||
mockSet: vi.fn(),
|
||||
mockWhere: vi.fn(),
|
||||
mockParseRequest: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@sim/db', () => ({
|
||||
db: { update: mockUpdate },
|
||||
}))
|
||||
|
||||
vi.mock('@sim/db/schema', () => ({
|
||||
copilotChats: {
|
||||
id: 'copilotChats.id',
|
||||
userId: 'copilotChats.userId',
|
||||
updatedAt: 'copilotChats.updatedAt',
|
||||
lastSeenAt: 'copilotChats.lastSeenAt',
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('drizzle-orm', () => ({
|
||||
and: vi.fn((...conditions: unknown[]) => ({ type: 'and', conditions })),
|
||||
eq: vi.fn((field: unknown, value: unknown) => ({ type: 'eq', field, value })),
|
||||
or: vi.fn((...conditions: unknown[]) => ({ type: 'or', conditions })),
|
||||
isNull: vi.fn((field: unknown) => ({ type: 'isNull', field })),
|
||||
lt: vi.fn((field: unknown, value: unknown) => ({ type: 'lt', field, value })),
|
||||
sql: vi.fn(() => ({ type: 'sql' })),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/copilot/request/http', () => copilotHttpMock)
|
||||
vi.mock('@/lib/api/server', () => ({ parseRequest: mockParseRequest }))
|
||||
vi.mock('@/lib/api/contracts/mothership-chats', () => ({ markMothershipChatReadContract: {} }))
|
||||
|
||||
import { POST } from '@/app/api/mothership/chats/read/route'
|
||||
|
||||
function createRequest() {
|
||||
return new NextRequest('http://localhost:3000/api/mothership/chats/read', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ chatId: 'chat-1' }),
|
||||
})
|
||||
}
|
||||
|
||||
describe('POST /api/mothership/chats/read', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
copilotHttpMockFns.mockAuthenticateCopilotRequestSessionOnly.mockResolvedValue({
|
||||
userId: 'user-1',
|
||||
isAuthenticated: true,
|
||||
})
|
||||
mockParseRequest.mockResolvedValue({ success: true, data: { body: { chatId: 'chat-1' } } })
|
||||
mockWhere.mockResolvedValue(undefined)
|
||||
mockSet.mockReturnValue({ where: mockWhere })
|
||||
mockUpdate.mockReturnValue({ set: mockSet })
|
||||
})
|
||||
|
||||
it('guards the lastSeenAt write with the unread predicate (only writes when unread)', async () => {
|
||||
const res = await POST(createRequest())
|
||||
expect(res.status).toBe(200)
|
||||
|
||||
expect(mockUpdate).toHaveBeenCalledTimes(1)
|
||||
const whereArg = mockWhere.mock.calls[0][0] as {
|
||||
type: string
|
||||
conditions: Array<{ type: string; conditions?: unknown[] }>
|
||||
}
|
||||
expect(whereArg.type).toBe('and')
|
||||
|
||||
const orClause = whereArg.conditions.find((c) => c.type === 'or')
|
||||
expect(orClause).toBeDefined()
|
||||
expect(orClause?.conditions).toEqual(
|
||||
expect.arrayContaining([
|
||||
{ type: 'isNull', field: 'copilotChats.lastSeenAt' },
|
||||
{ type: 'lt', field: 'copilotChats.lastSeenAt', value: 'copilotChats.updatedAt' },
|
||||
])
|
||||
)
|
||||
})
|
||||
|
||||
it('does not touch the database when unauthenticated', async () => {
|
||||
copilotHttpMockFns.mockAuthenticateCopilotRequestSessionOnly.mockResolvedValue({
|
||||
userId: null,
|
||||
isAuthenticated: false,
|
||||
})
|
||||
const res = await POST(createRequest())
|
||||
expect(res.status).toBe(401)
|
||||
expect(mockUpdate).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -1,7 +1,7 @@
|
||||
import { db } from '@sim/db'
|
||||
import { copilotChats } from '@sim/db/schema'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { and, eq, sql } from 'drizzle-orm'
|
||||
import { and, eq, isNull, lt, or, sql } from 'drizzle-orm'
|
||||
import { type NextRequest, NextResponse } from 'next/server'
|
||||
import { markMothershipChatReadContract } from '@/lib/api/contracts/mothership-chats'
|
||||
import { parseRequest } from '@/lib/api/server'
|
||||
@@ -28,7 +28,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
|
||||
await db
|
||||
.update(copilotChats)
|
||||
.set({ lastSeenAt: sql`GREATEST(${copilotChats.updatedAt}, NOW())` })
|
||||
.where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId)))
|
||||
.where(
|
||||
and(
|
||||
eq(copilotChats.id, chatId),
|
||||
eq(copilotChats.userId, userId),
|
||||
or(isNull(copilotChats.lastSeenAt), lt(copilotChats.lastSeenAt, copilotChats.updatedAt))
|
||||
)
|
||||
)
|
||||
|
||||
return NextResponse.json({ success: true })
|
||||
} catch (error) {
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
* Response: AdminListResponse<AdminAuditLog>
|
||||
*/
|
||||
|
||||
import { db } from '@sim/db'
|
||||
import { dbReplica } from '@sim/db'
|
||||
import { auditLog } from '@sim/db/schema'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { and, count, desc } from 'drizzle-orm'
|
||||
@@ -70,8 +70,8 @@ export const GET = withRouteHandler(
|
||||
const whereClause = conditions.length > 0 ? and(...conditions) : undefined
|
||||
|
||||
const [countResult, logs] = await Promise.all([
|
||||
db.select({ total: count() }).from(auditLog).where(whereClause),
|
||||
db
|
||||
dbReplica.select({ total: count() }).from(auditLog).where(whereClause),
|
||||
dbReplica
|
||||
.select()
|
||||
.from(auditLog)
|
||||
.where(whereClause)
|
||||
|
||||
@@ -21,7 +21,7 @@
|
||||
* Response: AdminSingleResponse<AdminOrganization & { memberId: string }>
|
||||
*/
|
||||
|
||||
import { db } from '@sim/db'
|
||||
import { db, dbReplica } from '@sim/db'
|
||||
import { member, organization, user } from '@sim/db/schema'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { count, eq } from 'drizzle-orm'
|
||||
@@ -70,8 +70,8 @@ export const GET = withRouteHandler(
|
||||
|
||||
try {
|
||||
const [countResult, organizations] = await Promise.all([
|
||||
db.select({ total: count() }).from(organization),
|
||||
db
|
||||
dbReplica.select({ total: count() }).from(organization),
|
||||
dbReplica
|
||||
.select({
|
||||
id: organization.id,
|
||||
name: organization.name,
|
||||
|
||||
@@ -22,12 +22,6 @@ export interface FileMetadataInsertOptions {
|
||||
id?: string
|
||||
}
|
||||
|
||||
interface FileMetadataQueryOptions {
|
||||
context?: StorageContext
|
||||
workspaceId?: string
|
||||
userId?: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Insert file metadata into workspaceFiles table
|
||||
* Handles duplicate key errors gracefully by returning existing record
|
||||
@@ -229,34 +223,6 @@ export async function getFileMetadataById(
|
||||
return record ?? null
|
||||
}
|
||||
|
||||
/**
|
||||
* Get file metadata by context with optional workspaceId/userId filters
|
||||
*/
|
||||
async function getFileMetadataByContext(
|
||||
context: StorageContext,
|
||||
options?: FileMetadataQueryOptions & { includeDeleted?: boolean }
|
||||
): Promise<FileMetadataRecord[]> {
|
||||
const conditions = [eq(workspaceFiles.context, context)]
|
||||
|
||||
if (options?.workspaceId) {
|
||||
conditions.push(eq(workspaceFiles.workspaceId, options.workspaceId))
|
||||
}
|
||||
|
||||
if (options?.userId) {
|
||||
conditions.push(eq(workspaceFiles.userId, options.userId))
|
||||
}
|
||||
|
||||
if (!options?.includeDeleted) {
|
||||
conditions.push(isNull(workspaceFiles.deletedAt))
|
||||
}
|
||||
|
||||
return db
|
||||
.select()
|
||||
.from(workspaceFiles)
|
||||
.where(conditions.length > 1 ? and(...conditions) : conditions[0])
|
||||
.orderBy(workspaceFiles.uploadedAt)
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete file metadata by key
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mockUpdate, mockSet, mockWhere, sqlCalls } = vi.hoisted(() => ({
|
||||
mockUpdate: vi.fn(),
|
||||
mockSet: vi.fn(),
|
||||
mockWhere: vi.fn(),
|
||||
sqlCalls: [] as Array<{ values: unknown[] }>,
|
||||
}))
|
||||
|
||||
vi.mock('@sim/db', () => ({ db: { update: mockUpdate } }))
|
||||
vi.mock('@sim/db/schema', () => ({
|
||||
webhook: {
|
||||
id: 'webhook.id',
|
||||
providerConfig: 'webhook.providerConfig',
|
||||
updatedAt: 'webhook.updatedAt',
|
||||
},
|
||||
account: {},
|
||||
credentialSet: {},
|
||||
workflow: {},
|
||||
workflowDeploymentVersion: {},
|
||||
}))
|
||||
vi.mock('drizzle-orm', () => ({
|
||||
sql: (_strings: readonly string[], ...values: unknown[]) => {
|
||||
const node = { values }
|
||||
sqlCalls.push(node)
|
||||
return node
|
||||
},
|
||||
and: vi.fn(),
|
||||
eq: vi.fn((field: unknown, value: unknown) => ({ field, value })),
|
||||
isNull: vi.fn(),
|
||||
ne: vi.fn(),
|
||||
or: vi.fn(),
|
||||
}))
|
||||
vi.mock('@/lib/billing', () => ({ isOrganizationOnTeamOrEnterprisePlan: vi.fn() }))
|
||||
vi.mock('@/app/api/auth/oauth/utils', () => ({
|
||||
getOAuthToken: vi.fn(),
|
||||
refreshAccessTokenIfNeeded: vi.fn(),
|
||||
resolveOAuthAccountId: vi.fn(),
|
||||
}))
|
||||
vi.mock('@/triggers/constants', () => ({ MAX_CONSECUTIVE_FAILURES: 5 }))
|
||||
|
||||
import { updateWebhookProviderConfig } from '@/lib/webhooks/polling/utils'
|
||||
|
||||
const logger = { error: vi.fn() } as never
|
||||
|
||||
function allInterpolatedValues(): unknown[] {
|
||||
return sqlCalls.flatMap((c) => c.values)
|
||||
}
|
||||
|
||||
describe('updateWebhookProviderConfig (atomic jsonb merge)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
sqlCalls.length = 0
|
||||
mockWhere.mockResolvedValue(undefined)
|
||||
mockSet.mockReturnValue({ where: mockWhere })
|
||||
mockUpdate.mockReturnValue({ set: mockSet })
|
||||
})
|
||||
|
||||
it('merges defined keys (null preserved) and removes undefined keys', async () => {
|
||||
await updateWebhookProviderConfig(
|
||||
'wh-1',
|
||||
{ historyId: 'h1', cleared: undefined, nulled: null },
|
||||
logger
|
||||
)
|
||||
|
||||
expect(mockUpdate).toHaveBeenCalledTimes(1)
|
||||
expect(allInterpolatedValues()).toContain(JSON.stringify({ historyId: 'h1', nulled: null }))
|
||||
expect(allInterpolatedValues()).toContainEqual(['cleared'])
|
||||
})
|
||||
|
||||
it('uses merge only (no key-removal expression) when nothing is undefined', async () => {
|
||||
await updateWebhookProviderConfig('wh-1', { historyId: 'h1' }, logger)
|
||||
|
||||
expect(allInterpolatedValues()).toContain(JSON.stringify({ historyId: 'h1' }))
|
||||
expect(allInterpolatedValues().some((v) => Array.isArray(v))).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -157,16 +157,19 @@ export async function updateWebhookProviderConfig(
|
||||
logger: Logger
|
||||
): Promise<void> {
|
||||
try {
|
||||
const result = await db.select().from(webhook).where(eq(webhook.id, webhookId))
|
||||
const existingConfig = (result[0]?.providerConfig as Record<string, unknown>) || {}
|
||||
const defined: Record<string, unknown> = {}
|
||||
const removedKeys: string[] = []
|
||||
for (const [key, value] of Object.entries(configUpdates)) {
|
||||
if (value === undefined) removedKeys.push(key)
|
||||
else defined[key] = value
|
||||
}
|
||||
|
||||
const merged = sql`COALESCE(${webhook.providerConfig}, '{}'::jsonb) || ${JSON.stringify(defined)}::jsonb`
|
||||
|
||||
await db
|
||||
.update(webhook)
|
||||
.set({
|
||||
providerConfig: {
|
||||
...existingConfig,
|
||||
...configUpdates,
|
||||
} as Record<string, unknown>,
|
||||
providerConfig: removedKeys.length > 0 ? sql`(${merged}) - ${removedKeys}::text[]` : merged,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(webhook.id, webhookId))
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
-- Generated by drizzle-kit, then converted to CONCURRENTLY per scripts/migrate.ts (plain
|
||||
-- CREATE/DROP INDEX takes ACCESS EXCLUSIVE for the whole op, write-locking the table).
|
||||
--
|
||||
-- ADD workflow_execution_logs (workspace_id, started_at DESC NULLS LAST, id DESC): serves the
|
||||
-- logs-list query (lib/logs/list-logs.ts, app/api/v1/logs, app/api/logs/export) ordered by
|
||||
-- started_at DESC with an id tiebreaker as an index-only scan that early-exits under LIMIT,
|
||||
-- replacing an in-memory sort over the matched rows. The existing
|
||||
-- workflow_execution_logs_workspace_started_at_idx is intentionally KEPT — it still serves the
|
||||
-- ascending/range log queries that this DESC index does not cover.
|
||||
--
|
||||
-- DROP three redundant indexes, each a left-prefix of an existing superset (equality-prefix only,
|
||||
-- no ordering subtlety; no query, foreign key, or constraint depends on the narrow index):
|
||||
-- permission_group_workspace_group_id_idx ⊂ permission_group_workspace_group_workspace_unique
|
||||
-- user_table_rows_table_id_idx ⊂ user_table_rows_table_order_key_idx (+ others)
|
||||
-- workspace_byok_workspace_idx ⊂ workspace_byok_workspace_provider_idx
|
||||
--
|
||||
-- The embedded COMMIT drops out of drizzle's batch transaction; everything below runs in
|
||||
-- autocommit (CONCURRENTLY cannot run in a transaction) and is idempotent for safe replay.
|
||||
COMMIT;--> statement-breakpoint
|
||||
SET lock_timeout = 0;--> statement-breakpoint
|
||||
CREATE INDEX CONCURRENTLY IF NOT EXISTS "workflow_execution_logs_workspace_started_at_id_desc_idx" ON "workflow_execution_logs" USING btree ("workspace_id","started_at" DESC NULLS LAST,"id" DESC);--> statement-breakpoint
|
||||
DROP INDEX CONCURRENTLY IF EXISTS "permission_group_workspace_group_id_idx";--> statement-breakpoint
|
||||
DROP INDEX CONCURRENTLY IF EXISTS "user_table_rows_table_id_idx";--> statement-breakpoint
|
||||
DROP INDEX CONCURRENTLY IF EXISTS "workspace_byok_workspace_idx";--> statement-breakpoint
|
||||
SET lock_timeout = '5s';
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1667,6 +1667,13 @@
|
||||
"when": 1781554278388,
|
||||
"tag": "0238_workspace_scoped_permission_groups",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 239,
|
||||
"version": "7",
|
||||
"when": 1781659761380,
|
||||
"tag": "0239_wel_logs_desc_index_and_redundant_drops",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -372,6 +372,9 @@ export const workflowExecutionLogs = pgTable(
|
||||
table.workspaceId,
|
||||
table.startedAt
|
||||
),
|
||||
workspaceStartedAtIdDescIdx: index(
|
||||
'workflow_execution_logs_workspace_started_at_id_desc_idx'
|
||||
).on(table.workspaceId, sql`${table.startedAt} DESC NULLS LAST`, sql`${table.id} DESC`),
|
||||
workspaceCostTotalIdx: index('workflow_execution_logs_workspace_cost_total_idx').on(
|
||||
table.workspaceId,
|
||||
table.costTotal
|
||||
@@ -564,7 +567,6 @@ export const workspaceBYOKKeys = pgTable(
|
||||
table.workspaceId,
|
||||
table.providerId
|
||||
),
|
||||
workspaceIdx: index('workspace_byok_workspace_idx').on(table.workspaceId),
|
||||
})
|
||||
)
|
||||
|
||||
@@ -2975,9 +2977,6 @@ export const permissionGroupWorkspace = pgTable(
|
||||
createdAt: timestamp('created_at').notNull().defaultNow(),
|
||||
},
|
||||
(table) => ({
|
||||
permissionGroupIdIdx: index('permission_group_workspace_group_id_idx').on(
|
||||
table.permissionGroupId
|
||||
),
|
||||
workspaceIdIdx: index('permission_group_workspace_workspace_id_idx').on(table.workspaceId),
|
||||
groupWorkspaceUnique: uniqueIndex('permission_group_workspace_group_workspace_unique').on(
|
||||
table.permissionGroupId,
|
||||
@@ -3193,7 +3192,6 @@ export const userTableRows = pgTable(
|
||||
createdBy: text('created_by').references(() => user.id, { onDelete: 'set null' }),
|
||||
},
|
||||
(table) => ({
|
||||
tableIdIdx: index('user_table_rows_table_id_idx').on(table.tableId),
|
||||
/**
|
||||
* Tenant-scoped containment index (requires the `btree_gin` extension,
|
||||
* created in migration 0232). A plain GIN on `data` matches `@>` candidates
|
||||
|
||||
Reference in New Issue
Block a user