Skip to content

LCORE-1249: PostgreSQL benchmarks#1152

Merged
tisnik merged 7 commits intolightspeed-core:mainfrom
tisnik:lcore-1249-pgvector-benchmarks
Feb 15, 2026
Merged

LCORE-1249: PostgreSQL benchmarks#1152
tisnik merged 7 commits intolightspeed-core:mainfrom
tisnik:lcore-1249-pgvector-benchmarks

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Feb 15, 2026

Description

LCORE-1249: PostgreSQL benchmarks

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1249

Summary by CodeRabbit

  • Tests
    • Reorganized and standardized database benchmark test naming conventions for improved clarity and better distinction between supported database implementations
    • Added comprehensive PostgreSQL performance benchmarks covering all conversation storage, updates, listing, and retrieval operations across multiple data size scenarios, enabling direct performance comparison and validation between SQLite and PostgreSQL database backends

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 15, 2026

Walkthrough

Benchmark tests in tests/benchmarks/test_app_database.py are refactored to explicitly distinguish database backends. Existing SQLite tests are prefixed with sqlite_ and new PostgreSQL equivalents with postgres_ prefix, covering store, update, list, and retrieve operations across multiple database sizes.

Changes

Cohort / File(s) Summary
Database Benchmark Tests
tests/benchmarks/test_app_database.py
24 SQLite benchmark tests renamed with sqlite_ prefix; 24 new PostgreSQL benchmark tests added with postgres_ prefix mirroring SQLite variants. Covers store, update, list (all/per-user), and retrieve operations across empty, small, middle, and large database sizes. PostgreSQL tests use postgres_database fixture instead of default database fixture.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'LCORE-1249: PostgreSQL benchmarks' clearly and directly summarizes the main change: adding PostgreSQL benchmarks as evidenced by the renaming of SQLite tests to include 'sqlite' prefix and addition of parallel 'postgres' benchmark test variants.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/benchmarks/test_app_database.py (1)

734-735: ⚠️ Potential issue | 🟠 Major

Bug: float division produces user IDs that never match stored records.

Line 735 uses / (float division), so str(records_to_insert / 2) yields "50.0" instead of "50". Since records are stored with str(id) from range() (producing "0", "1", …), the queried user_id will never match, and every "list for one user" benchmark silently measures an empty-result query on non-empty databases.

The analogous retrieve_conversation helpers on lines 821 and 909–910 correctly use //.

🐛 Proposed fix
-        user_id = str(records_to_insert / 2)
+        user_id = str(records_to_insert // 2)
🧹 Nitpick comments (1)
tests/benchmarks/test_app_database.py (1)

981-1350: Consider reducing duplication with pytest.mark.parametrize or a test factory.

The 24 new PostgreSQL test functions are structurally identical to the 24 SQLite tests — only the fixture differs. This doubles the maintenance surface (~360 lines). A common pattern is to parametrize over a fixture name using request.getfixturevalue:

`@pytest.mark.parametrize`("db_fixture", ["sqlite_database", "postgres_database"])
def test_store_new_user_conversations_empty_db(request, db_fixture, benchmark):
    request.getfixturevalue(db_fixture)
    benchmark_store_new_user_conversations(benchmark, 0)

This would collapse each pair into a single function and make it trivial to add new backends later.

@tisnik tisnik merged commit aab14c5 into lightspeed-core:main Feb 15, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant