From a16950a8f94dc17ff692b7addfebfe69610083a0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 5 Sep 2025 21:43:54 +0000 Subject: [PATCH] Implement database-aware revert optimization - Add getDeployedModules() method to query launchql_migrate.changes table - Make resolveWorkspaceExtensionDependencies() async with optional deployment filtering - Update revert() method to filter workspace extensions by deployment status - Fix test file to handle async resolveWorkspaceExtensionDependencies() calls - Add comprehensive REVERT.md documentation This optimization prevents recursive revert from processing undeployed modules, significantly improving performance for large workspaces. Co-Authored-By: Dan Lynch --- REVERT.md | 321 ++++++++++++++++++ ...kspace-extensions-dependency-order.test.ts | 24 +- packages/core/src/core/class/launchql.ts | 52 ++- 3 files changed, 375 insertions(+), 22 deletions(-) create mode 100644 REVERT.md diff --git a/REVERT.md b/REVERT.md new file mode 100644 index 000000000..7e977663a --- /dev/null +++ b/REVERT.md @@ -0,0 +1,321 @@ +# LaunchQL Revert Optimization Plan + +## Problem Statement + +The current recursive revert functionality in LaunchQL has a performance issue where it processes ALL modules in the workspace regardless of their deployment status. When using the `--recursive` option, the system calls `resolveWorkspaceExtensionDependencies()` which returns every module in the workspace, leading to unnecessary processing of modules that were never deployed to the database. + +### Current Behavior + +When executing `lql revert --recursive`, the system: + +1. Calls `resolveWorkspaceExtensionDependencies()` to get ALL workspace modules +2. Processes each module in reverse dependency order +3. Often skips many modules that were never deployed, wasting time on: + - File system operations + - Database connection attempts + - Unnecessary logging and error handling + +### Impact + +- Significantly slower revert operations in large workspaces +- Confusing output with many "skipping" messages for undeployed modules +- Unnecessary resource consumption (I/O, database connections) +- Poor user experience during rollback operations + +## Proposed Solution + +Implement database-aware filtering before dependency resolution by: + +1. Querying the `launchql_migrate` database to determine which modules are actually deployed +2. Filtering the workspace extension list to only include deployed modules +3. Processing only modules that have deployment history + +### Benefits + +- **Performance**: Dramatically reduces processing time for large workspaces +- **Clarity**: Eliminates confusing "skipping" messages for undeployed modules +- **Efficiency**: Reduces unnecessary database queries and file operations +- **Accuracy**: Maintains exact same behavior for actually deployed modules + +## Implementation Plan + +### Code Locations Requiring Changes + +#### Primary Location: LaunchQLPackage.revert() Method + +**File**: `packages/core/src/core/class/launchql.ts` +**Lines**: 1052-1152 +**Method**: `LaunchQLPackage.revert()` + +**Current problematic code** (lines 1067-1075): +```typescript +if (name === null) { + // When name is null, revert ALL modules in the workspace + extensionsToRevert = this.resolveWorkspaceExtensionDependencies(); +} else { + // Always use workspace-wide resolution in recursive mode + // This ensures all dependent modules are reverted before their dependencies. + const workspaceExtensions = this.resolveWorkspaceExtensionDependencies(); + extensionsToRevert = truncateExtensionsToTarget(workspaceExtensions, name); +} +``` + +#### Supporting Functions + +**1. resolveWorkspaceExtensionDependencies()** +- **File**: `packages/core/src/core/class/launchql.ts` +- **Lines**: 841-865 +- **Current**: Returns ALL workspace modules regardless of deployment status +- **Needs**: Optional database filtering capability + +**2. truncateExtensionsToTarget()** +- **File**: `packages/core/src/core/class/launchql.ts` +- **Lines**: 80-94 +- **Current**: Truncates from target module in dependency order +- **Needs**: Work with pre-filtered deployed modules list + +### Database Integration Points + +#### Existing Database Schema + +**Schema**: `launchql_migrate` +**File**: `packages/core/src/migrate/sql/schema.sql` + +**Key Tables**: +- `launchql_migrate.changes` - Tracks deployed changes per package +- `launchql_migrate.packages` - Tracks registered packages + +#### Existing Database Procedures + +**File**: `packages/core/src/migrate/sql/procedures.sql` + +**Key Function**: `launchql_migrate.deployed_changes()` +- **Lines**: 198-207 +- **Purpose**: Lists deployed changes, optionally filtered by package +- **Returns**: `(package TEXT, change_name TEXT, deployed_at TIMESTAMPTZ)` + +#### Existing Client Methods + +**File**: `packages/core/src/migrate/client.ts` + +**Key Method**: `LaunchQLMigrate.getDeployedChanges()` +- **Lines**: 605-630 +- **Purpose**: Gets all deployed changes for a package +- **Usage**: Can be adapted to get list of deployed packages + +## Implementation Strategy + +### Option A: Modify Existing Methods (Recommended) + +#### 1. Enhance resolveWorkspaceExtensionDependencies() + +```typescript +public async resolveWorkspaceExtensionDependencies( + opts?: { filterDeployed?: boolean; pgConfig?: PgConfig } +): Promise<{ resolved: string[]; external: string[] }> { + const modules = this.getModuleMap(); + const allModuleNames = Object.keys(modules); + + if (allModuleNames.length === 0) { + return { resolved: [], external: [] }; + } + + // Create virtual module that depends on all workspace modules + const virtualModuleName = '_virtual/workspace'; + const virtualModuleMap = { + ...modules, + [virtualModuleName]: { + requires: allModuleNames + } + }; + + const { resolved, external } = resolveExtensionDependencies(virtualModuleName, virtualModuleMap); + + let filteredResolved = resolved.filter((moduleName: string) => moduleName !== virtualModuleName); + + // NEW: Filter by deployment status if requested + if (opts?.filterDeployed && opts?.pgConfig) { + const deployedModules = await this.getDeployedModules(opts.pgConfig); + filteredResolved = filteredResolved.filter(module => deployedModules.has(module)); + } + + return { + resolved: filteredResolved, + external: external + }; +} +``` + +#### 2. Add Helper Method: getDeployedModules() + +```typescript +private async getDeployedModules(pgConfig: PgConfig): Promise> { + try { + const client = new LaunchQLMigrate(pgConfig); + await client.initialize(); + + // Query all deployed packages + const result = await client.pool.query(` + SELECT DISTINCT package + FROM launchql_migrate.changes + WHERE deployed_at IS NOT NULL + `); + + return new Set(result.rows.map(row => row.package)); + } catch (error: any) { + // If schema doesn't exist or other DB errors, assume no deployments + if (error.code === '42P01' || error.code === '3F000') { + return new Set(); + } + throw error; + } +} +``` + +#### 3. Update revert() Method + +**Replace lines 1067-1075** with: + +```typescript +if (name === null) { + // When name is null, revert ALL deployed modules in the workspace + extensionsToRevert = await this.resolveWorkspaceExtensionDependencies({ + filterDeployed: true, + pgConfig: opts.pg as PgConfig + }); +} else { + // Always use workspace-wide resolution in recursive mode, but filter to deployed modules + const workspaceExtensions = await this.resolveWorkspaceExtensionDependencies({ + filterDeployed: true, + pgConfig: opts.pg as PgConfig + }); + extensionsToRevert = truncateExtensionsToTarget(workspaceExtensions, name); +} +``` + +### Option B: Add New Method (Alternative) + +#### Add getDeployedWorkspaceExtensions() Method + +```typescript +private async getDeployedWorkspaceExtensions( + opts: LaunchQLOptions +): Promise<{ resolved: string[]; external: string[] }> { + // Get all workspace extensions + const allExtensions = this.resolveWorkspaceExtensionDependencies(); + + // Get deployed modules from database + const deployedModules = await this.getDeployedModules(opts.pg as PgConfig); + + // Filter to only deployed modules + return { + resolved: allExtensions.resolved.filter(module => deployedModules.has(module)), + external: allExtensions.external + }; +} +``` + +## Error Handling Considerations + +### Database Connection Issues +- Handle cases where `launchql_migrate` schema doesn't exist +- Gracefully handle database connection errors +- Fall back to current behavior if database queries fail + +### Backward Compatibility +- Maintain existing behavior for non-recursive reverts +- Ensure existing API contracts are preserved +- Add optional parameters rather than breaking changes + +### Edge Cases +- Empty database (no deployments) - should return empty set +- Missing schema - treat as no deployments +- Network/connection errors - log warning and fall back to current behavior + +## Testing Strategy + +### Unit Tests +- Test `getDeployedModules()` with various database states +- Test `resolveWorkspaceExtensionDependencies()` with filtering enabled/disabled +- Test error handling for missing schema and connection issues + +### Integration Tests +- Create workspace with mix of deployed and undeployed modules +- Verify recursive revert only processes deployed modules +- Test with empty database and missing schema +- Verify non-recursive reverts remain unchanged + +### Performance Tests +- Measure improvement in large workspaces with many undeployed modules +- Compare processing time before and after optimization +- Verify memory usage doesn't increase significantly + +## Migration Considerations + +### Existing Deployments +- Optimization works with existing `launchql_migrate` schema +- No database migrations required +- Compatible with imported Sqitch deployments + +### Configuration +- No configuration changes required +- Optimization is automatic when using recursive revert +- Maintains backward compatibility + +## Files Requiring Modification + +### Primary Files +1. **`packages/core/src/core/class/launchql.ts`** + - Modify `resolveWorkspaceExtensionDependencies()` method + - Add `getDeployedModules()` helper method + - Update `revert()` method to use database filtering + +### Supporting Files (if needed) +2. **`packages/core/src/migrate/client.ts`** + - Potentially add helper methods for querying deployed packages + - Enhance error handling for schema detection + +### Test Files (new) +3. **`packages/core/__tests__/core/revert-optimization.test.ts`** + - Unit tests for new functionality + - Integration tests for recursive revert optimization + +## Success Metrics + +### Performance Improvements +- **Target**: 50-80% reduction in processing time for workspaces with >10 undeployed modules +- **Measurement**: Time from revert command start to completion +- **Baseline**: Current recursive revert performance + +### User Experience +- **Elimination**: No more "skipping" messages for undeployed modules +- **Clarity**: Only deployed modules appear in revert output +- **Speed**: Faster feedback during rollback operations + +### Reliability +- **Compatibility**: 100% backward compatibility with existing functionality +- **Error Handling**: Graceful degradation when database is unavailable +- **Accuracy**: Identical behavior for actually deployed modules + +## Implementation Timeline + +### Phase 1: Core Implementation +1. Add `getDeployedModules()` helper method +2. Modify `resolveWorkspaceExtensionDependencies()` with optional filtering +3. Update `revert()` method to use database-aware filtering + +### Phase 2: Testing & Validation +1. Create comprehensive unit tests +2. Add integration tests with various deployment scenarios +3. Performance testing and benchmarking + +### Phase 3: Documentation & Rollout +1. Update CLI help text and documentation +2. Add migration notes for existing users +3. Monitor performance improvements in production + +## Conclusion + +This optimization addresses a significant performance bottleneck in LaunchQL's recursive revert functionality. By leveraging the existing `launchql_migrate` database schema to filter modules before processing, we can dramatically improve performance while maintaining full backward compatibility and accuracy. + +The implementation follows existing code patterns and leverages already-available database infrastructure, making it a low-risk, high-impact improvement to the user experience. diff --git a/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts b/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts index d853e362b..545f6cba6 100644 --- a/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts +++ b/packages/core/__tests__/core/workspace-extensions-dependency-order.test.ts @@ -15,27 +15,27 @@ afterAll(() => { describe('getWorkspaceExtensionsInDependencyOrder', () => { describe('with existing fixtures', () => { - it('returns extensions in dependency order for simple-w-tags workspace', () => { + it('returns extensions in dependency order for simple-w-tags workspace', async () => { const workspacePath = fixture.getFixturePath('simple-w-tags'); const project = new LaunchQLPackage(workspacePath); - const result = project.resolveWorkspaceExtensionDependencies(); + const result = await project.resolveWorkspaceExtensionDependencies(); expect(result).toMatchSnapshot(); }); - it('returns extensions in dependency order for launchql workspace', () => { + it('returns extensions in dependency order for launchql workspace', async () => { const workspacePath = fixture.getFixturePath('launchql'); const project = new LaunchQLPackage(workspacePath); - const result = project.resolveWorkspaceExtensionDependencies(); + const result = await project.resolveWorkspaceExtensionDependencies(); expect(result).toMatchSnapshot(); }); - it('handles workspace with minimal modules', () => { + it('handles workspace with minimal modules', async () => { const workspacePath = fixture.getFixturePath('simple'); const project = new LaunchQLPackage(workspacePath); - const result = project.resolveWorkspaceExtensionDependencies(); + const result = await project.resolveWorkspaceExtensionDependencies(); expect(result).toMatchSnapshot(); expect(result).toHaveProperty('resolved'); @@ -44,22 +44,22 @@ describe('getWorkspaceExtensionsInDependencyOrder', () => { }); describe('with complex existing fixtures', () => { - it('returns extensions in dependency order for complex workspace', () => { + it('returns extensions in dependency order for complex workspace', async () => { const workspacePath = fixture.getFixturePath('launchql'); const project = new LaunchQLPackage(workspacePath); - const result = project.resolveWorkspaceExtensionDependencies(); + const result = await project.resolveWorkspaceExtensionDependencies(); expect(result).toMatchSnapshot(); expect(Array.isArray(result.resolved)).toBe(true); expect(Array.isArray(result.external)).toBe(true); }); - it('verifies dependency ordering properties', () => { + it('verifies dependency ordering properties', async () => { const workspacePath = fixture.getFixturePath('simple-w-tags'); const project = new LaunchQLPackage(workspacePath); - const result = project.resolveWorkspaceExtensionDependencies(); + const result = await project.resolveWorkspaceExtensionDependencies(); expect(result).toMatchSnapshot(); expect(result).toHaveProperty('resolved'); @@ -71,11 +71,11 @@ describe('getWorkspaceExtensionsInDependencyOrder', () => { expect(result.external.length).toBeGreaterThanOrEqual(0); }); - it('handles different fixture types consistently', () => { + it('handles different fixture types consistently', async () => { const workspacePath = fixture.getFixturePath('simple'); const project = new LaunchQLPackage(workspacePath); - const result = project.resolveWorkspaceExtensionDependencies(); + const result = await project.resolveWorkspaceExtensionDependencies(); expect(result).toMatchSnapshot(); expect(result.resolved.length).toBeGreaterThan(0); diff --git a/packages/core/src/core/class/launchql.ts b/packages/core/src/core/class/launchql.ts index 8364729f1..7bb20a207 100644 --- a/packages/core/src/core/class/launchql.ts +++ b/packages/core/src/core/class/launchql.ts @@ -838,7 +838,27 @@ export class LaunchQLPackage { // ──────────────── Package Operations ──────────────── - public resolveWorkspaceExtensionDependencies(): { resolved: string[]; external: string[] } { + /** + * Get the set of modules that have been deployed to the database + */ + private async getDeployedModules(pgConfig: PgConfig): Promise> { + try { + const client = new LaunchQLMigrate(pgConfig); + await client.initialize(); + + const status = await client.status(); + return new Set(status.map(s => s.package)); + } catch (error: any) { + if (error.code === '42P01' || error.code === '3F000') { + return new Set(); + } + throw error; + } + } + + public async resolveWorkspaceExtensionDependencies( + opts?: { filterDeployed?: boolean; pgConfig?: PgConfig } + ): Promise<{ resolved: string[]; external: string[] }> { const modules = this.getModuleMap(); const allModuleNames = Object.keys(modules); @@ -857,9 +877,16 @@ export class LaunchQLPackage { const { resolved, external } = resolveExtensionDependencies(virtualModuleName, virtualModuleMap); - // Filter out the virtual module and return the result + let filteredResolved = resolved.filter((moduleName: string) => moduleName !== virtualModuleName); + + // Filter by deployment status if requested + if (opts?.filterDeployed && opts?.pgConfig) { + const deployedModules = await this.getDeployedModules(opts.pgConfig); + filteredResolved = filteredResolved.filter(module => deployedModules.has(module)); + } + return { - resolved: resolved.filter((moduleName: string) => moduleName !== virtualModuleName), + resolved: filteredResolved, external: external }; } @@ -919,7 +946,7 @@ export class LaunchQLPackage { if (name === null) { // When name is null, deploy ALL modules in the workspace - extensions = this.resolveWorkspaceExtensionDependencies(); + extensions = await this.resolveWorkspaceExtensionDependencies(); } else { const moduleProject = this.getModuleProject(name); extensions = moduleProject.getModuleExtensions(); @@ -1065,12 +1092,17 @@ export class LaunchQLPackage { let extensionsToRevert: { resolved: string[]; external: string[] }; if (name === null) { - // When name is null, revert ALL modules in the workspace - extensionsToRevert = this.resolveWorkspaceExtensionDependencies(); + // When name is null, revert ALL deployed modules in the workspace + extensionsToRevert = await this.resolveWorkspaceExtensionDependencies({ + filterDeployed: true, + pgConfig: opts.pg as PgConfig + }); } else { - // Always use workspace-wide resolution in recursive mode - // This ensures all dependent modules are reverted before their dependencies. - const workspaceExtensions = this.resolveWorkspaceExtensionDependencies(); + // Always use workspace-wide resolution in recursive mode, but filter to deployed modules + const workspaceExtensions = await this.resolveWorkspaceExtensionDependencies({ + filterDeployed: true, + pgConfig: opts.pg as PgConfig + }); extensionsToRevert = truncateExtensionsToTarget(workspaceExtensions, name); } @@ -1167,7 +1199,7 @@ export class LaunchQLPackage { if (name === null) { // When name is null, verify ALL modules in the workspace - extensions = this.resolveWorkspaceExtensionDependencies(); + extensions = await this.resolveWorkspaceExtensionDependencies(); } else { const moduleProject = this.getModuleProject(name); extensions = moduleProject.getModuleExtensions();