Database Performance Optimizations #4

akshayutture-augment · 2025-11-14T21:22:22Z

Test 7

Replicated from ai-code-review-evaluation/grafana-greptile#7

… deadlocks on MySQL (#80329) * Split subquery when cleaning annotations * update comment * Raise batch size, now that we pay attention to it * Iterate in batches * Separate cancellable batch implementation to allow for multi-statement callbacks, add overload for single-statement use * Use split-out utility in outer batching loop so it respects context cancellation * guard against empty queries * Use SQL parameters * Use same approach for tags * drop unused function * Work around parameter limit on sqlite for large batches * Bulk insert test data in DB * Refactor test to customise test data creation * Add test for catching SQLITE_MAX_VARIABLE_NUMBER limit * Turn annotation cleanup test to integration tests * lint --------- Co-authored-by: Sofia Papagiannaki <[email protected]>

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

akshayutture-augment · 2025-11-14T21:35:10Z

@greptileai

greptile-apps · 2025-11-14T21:40:06Z

Greptile Overview

Confidence Score: 2/5

This PR has critical performance concerns due to the 10x increase in cleanup frequency
The fetch-then-delete pattern is a solid improvement to prevent MySQL deadlocks, but two critical issues significantly lower the score: (1) the cleanup interval reduced from 10 to 1 minute will cause 10x more database load, and (2) incorrect log.Error usage will pollute error logs with routine operational messages
Pay close attention to pkg/services/cleanup/cleanup.go (cleanup interval change) and pkg/services/annotations/annotationsimpl/xorm_store.go (logging issues)

Important Files Changed

File Analysis

Filename	Score	Overview
pkg/services/annotations/annotationsimpl/xorm_store.go	2/5	Split cleanup into fetch-then-delete pattern to avoid MySQL deadlocks; added SQLite parameter limit handling; critical issue: using log.Error for non-error messages
pkg/services/cleanup/cleanup.go	1/5	Cleanup interval reduced from 10 minutes to 1 minute - 10x increase in execution frequency could significantly impact database performance
pkg/services/annotations/annotationsimpl/cleanup_test.go	4/5	Refactored tests for better isolation and batch insert performance; added test for large batch sizes with SQLite parameter limits

Sequence Diagram

sequenceDiagram
    participant CS as CleanUpService
    participant AC as AnnotationCleaner
    participant XR as xormRepositoryImpl
    participant DB as Database
    
    CS->>CS: ticker (1 minute interval)
    CS->>AC: Run(ctx, cfg)
    
    alt MaxAge cleanup
        AC->>XR: CleanAnnotations(ctx, cfg, annotationType)
        loop until done or cancelled
            XR->>DB: fetchIDs(table, condition)
            DB-->>XR: []int64 IDs
            alt SQLite with large batch
                XR->>XR: Build DELETE with inline IDs
            else
                XR->>XR: Build DELETE with placeholders
            end
            XR->>DB: deleteByIDs(table, ids)
            DB-->>XR: affected rows
            alt affected == 0
                XR-->>AC: totalAffected
            end
        end
    end
    
    alt MaxCount cleanup
        AC->>XR: CleanAnnotations(ctx, cfg, annotationType)
        loop until done or cancelled
            XR->>DB: fetchIDs with LIMIT/OFFSET
            DB-->>XR: []int64 IDs
            XR->>DB: deleteByIDs(table, ids)
            DB-->>XR: affected rows
        end
    end
    
    AC->>XR: CleanOrphanedAnnotationTags(ctx)
    loop until done or cancelled
        XR->>DB: fetchIDs(orphaned tags)
        DB-->>XR: []int64 IDs
        XR->>DB: deleteByIDs(annotation_tag, ids)
        DB-->>XR: affected rows
    end
    
    AC-->>CS: affected annotations, affected tags

greptile-apps

_{3 files reviewed, 7 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-11-14T21:40:02Z