-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(torii-core): add indices to speed up queries #2824
Conversation
WalkthroughOhayo, sensei! The pull request introduces performance-enhancing database indices for the Torii core SQL module. Two primary files are modified: the Rust source code in Changes
Sequence DiagramsequenceDiagram
participant SQL as SQL Migration
participant Core as Torii Core
SQL->>Core: Define new indices
Core->>Core: Conditionally create indices during schema upgrade
Core-->>SQL: Apply indices to database schema
Possibly Related PRs
Suggested Reviewers
Sensei, the indices are ready to boost our query performance! Ohayo! 🚀🔍 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/torii/core/src/sql/mod.rs (2)
769-776
: Ohayo! Good addition of indices on foreign key columns, sensei!Adding indices on foreign key columns
internal_entity_id
andinternal_event_message_id
is a good practice as it will improve JOIN performance. Consider combining these indices since they're often used together in queries.Consider creating a composite index:
- indices.push(format!( - "CREATE INDEX IF NOT EXISTS [idx_{table_id}_internal_entity_id] ON [{table_id}] \ - ([internal_entity_id]);" - )); - indices.push(format!( - "CREATE INDEX IF NOT EXISTS [idx_{table_id}_internal_event_message_id] ON [{table_id}] \ - ([internal_event_message_id]);" - )); + indices.push(format!( + "CREATE INDEX IF NOT EXISTS [idx_{table_id}_internal_ids] ON [{table_id}] \ + ([internal_entity_id], [internal_event_message_id]);" + ));
769-776
: Consider monitoring index usage and performance impactWhile these indices will improve query performance, they also:
- Increase storage space requirements
- Add overhead to write operations
- Need maintenance (rebuilding/reindexing)
Consider implementing monitoring to track:
- Index usage statistics
- Query performance improvements
- Write operation impact
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/torii/core/src/sql/mod.rs
(1 hunks)crates/torii/migrations/20241219104134_indices.sql
(1 hunks)
🔇 Additional comments (1)
crates/torii/migrations/20241219104134_indices.sql (1)
1-9
: Ohayo, sensei! The indices look well-structured.
The indices are appropriately named and placed on columns that are likely used in WHERE clauses or JOINs. This should improve query performance for operations involving these columns.
Let's verify if these columns are indeed frequently used in queries:
✅ Verification successful
Ohayo, sensei! The indices are well-justified by the codebase usage.
The evidence from the codebase strongly supports the need for these indices:
updated_at
: Used in WHERE clauses for querying entities by timestamp (found incrates/torii/grpc/src/server/mod.rs
)keys
: Extensively used in WHERE clauses with REGEXP for filtering (found in multiple files, e.g.,crates/torii/grpc/src/server/mod.rs
)event_id
: Used in ORDER BY clauses for sorting results (found in multiple queries acrossgrpc/src/server/mod.rs
andcore/src/model.rs
)executed_at
: Part of the core schema and used in historical tracking
The indices will improve query performance for these frequently used columns in WHERE clauses, JOIN conditions, and ORDER BY operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for SQL queries using these columns
rg -i "where.*updated_at|join.*updated_at" --type rust
rg -i "where.*event_id|join.*event_id" --type rust
rg -i "where.*executed_at|join.*executed_at" --type rust
rg -i "where.*keys|join.*keys" --type rust
Length of output: 1001
Script:
#!/bin/bash
# Let's check for any other potential usages of these columns in SQL queries
# that might not be in WHERE/JOIN clauses
# Search for any SQL-like usage of these columns
rg -i "updated_at|event_id|executed_at|keys" --type sql -A 3 -B 3
rg -i "select.*updated_at|select.*event_id|select.*executed_at|select.*keys" --type rust
rg -i "order by.*updated_at|order by.*event_id|order by.*executed_at|order by.*keys" --type rust
# Also check table definitions to understand the role of these columns
rg -i "create table.*entities|create table.*event_messages_historical" --type sql -A 10
Length of output: 27217
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/torii/core/src/sql/mod.rs (1)
769-776
: Ohayo, sensei! The indices look good and will improve query performance!The addition of indices on
internal_entity_id
andinternal_event_message_id
columns is a solid performance optimization. These indices will significantly speed up lookups and joins involving these columns.A few positive observations:
- Using
IF NOT EXISTS
prevents errors on repeated executions- Index naming follows best practices
- The indices are properly scoped to their respective tables
For optimal performance, consider monitoring the usage patterns of these indices once they're in production. Tools like
EXPLAIN QUERY PLAN
can help verify that the indices are being utilized effectively.
Summary by CodeRabbit
New Features
internal_entity_id
,internal_event_message_id
, and other key columns.entities
andevent_messages_historical
tables for faster data retrieval.Bug Fixes