Database Performance Optimizations

[malinosqui]

Here is a precise description of the pull request based on the provided code changes:

Summary

This PR introduces database performance optimizations and stability improvements for the background cleanup processes, specifically targeting how annotations and orphaned tags are deleted. It resolves potential database deadlocks, fixes a crash related to SQLite limits, and increases the frequency of the cleanup job.

Key Changes

• Deadlock Prevention in Annotation Cleanup:
  • Redesigned the CleanAnnotations and CleanOrphanedAnnotationTags logic. Instead of using a single complex DELETE statement with batched sub-queries (which caused deadlocks with concurrent inserts, particularly in MySQL), the process now fetches the target IDs into memory first and then deletes them in batches. This allows database locks to flush between operations.
• SQLite Parameter Limit Fix:
  • Added specific handling for SQLite databases during batch deletions. If the cleanup batch size exceeds SQLite's maximum parameter limit (999), the query now safely formats the IDs directly into the SQL statement to prevent execution failures.
• Increased Cleanup Frequency:
  • Reduced the interval of the background cleanup job from running every 10 minutes to every 1 minute. This ensures stale data is removed more frequently in smaller, more manageable batches.
• Test Suite Optimizations:
  • Significantly sped up the annotation cleanup tests by utilizing batch inserts (InsertMulti) instead of single-row inserts when generating test data.
  • Renamed database-heavy tests to indicate they are integration tests and added logic to skip them when running tests in short mode.
  • Added a specific test case to verify the SQLite variable limit fix for large batch sizes.

[malinosqui]

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration

Interacting with Kody

• Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

• Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

• Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Bug	✅
Performance	✅
Security	✅
Business Logic	✅

Access your configuration settings here.

​

[malinosqui]

O(N²) memory allocation bottleneck exists in the SQLite string concatenation loop because fmt.Sprintf generates a new string on every iteration. Use strings.Join or strings.Builder to construct the comma-separated values in linear time and avoid Out-Of-Memory crashes.

var strIDs []string
for _, id := range ids {
	strIDs = append(strIDs, strconv.FormatInt(id, 10))
}
sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, strings.Join(strIDs, ","))

Prompt for LLM

File pkg/services/annotations/annotationsimpl/xorm_store.go:

Line 609 to 613:

WHAT: The SQLite string concatenation loop creates an O(N²) memory allocation bottleneck. WHY: Using `fmt.Sprintf` inside a loop generates a new string on every iteration. For large batch sizes (e.g., >10,000), this allocates hundreds of megabytes or even gigabytes of string garbage, risking Out-Of-Memory crashes and massive GC spikes. HOW: Use a string slice and `strings.Join`, or `strings.Builder` to construct the comma-separated values in linear time.

Suggested Code:

		var strIDs []string
		for _, id := range ids {
			strIDs = append(strIDs, strconv.FormatInt(id, 10))
		}
		sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, strings.Join(strIDs, ","))

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[malinosqui]

Parameter limit violation in PostgreSQL and MySQL occurs because asAny(ids) creates a parameter for each ID, exceeding the 65,535 limit. Chunk the ids slice or cap AnnotationCleanupJobBatchSize to prevent driver errors from breaking the cleanup routine.

Prompt for LLM

File pkg/services/annotations/annotationsimpl/xorm_store.go:

Line 615 to 618:

WHAT: The new slice-based placeholder generation imposes a hard limit of 65,535 on the batch size for PostgreSQL and MySQL. WHY: Previously, the batch size was enforced via a `LIMIT` subquery which did not use placeholders and had no upper bound. Now, `asAny(ids)` creates a parameter for each ID. If an administrator configures `AnnotationCleanupJobBatchSize` > 65535, the cleanup job will perpetually fail with a driver error ('too many parameters' or 'prepared statement contains too many placeholders'), breaking the cleanup routine. HOW: Chunk the slice of IDs into smaller batches (e.g., 65000 elements) before executing the query, or cap the configured batch size at runtime.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[malinosqui]

Unintentional r.log.Error calls in the cleanup routines, including MaxCount and OrphanedTags blocks, spam production logs with large ids arrays and cause false alarms. Remove these statements or change the level to r.log.Debug to prevent log bloat.

ids, err := r.fetchIDs(ctx, "annotation", cond)
if err != nil {
	return 0, err
}

x, y := r.deleteByIDs(ctx, "annotation", ids)
return x, y

Prompt for LLM

File pkg/services/annotations/annotationsimpl/xorm_store.go:

Line 531 to 539:

WHAT: Debug statements using `r.log.Error` were accidentally left in the cleanup routines. WHY: Because the cleanup job runs every 1 minute, it will spam production logs with unconditional ERROR-level messages and format large arrays of IDs even when no cleanup is needed, causing false alarms and log bloat. HOW: Remove these log statements or change them to `r.log.Debug` (Note: this also applies to the similar log lines in the MaxCount and OrphanedTags blocks).

Suggested Code:

			ids, err := r.fetchIDs(ctx, "annotation", cond)
			if err != nil {
				return 0, err
			}

			x, y := r.deleteByIDs(ctx, "annotation", ids)
			return x, y

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​