Skip to content

Conversation

@adamdelman
Copy link
Contributor

@adamdelman adamdelman commented Jul 21, 2025

📝 Description

This PR refactors and enhances the database partitioning system for alert activations to improve cross-database compatibility, robustness, and maintainability. The changes establish a cleaner architecture for partition management while adding full PostgreSQL support alongside the existing MySQL implementation.

Key improvements:

  • Cross-database support: Added native PostgreSQL partitioning alongside MySQL, with SQLite gracefully skipping partition operations
  • Explicit partition keys: Introduced a dedicated partition_key integer column computed from activation_time, enabling consistent partitioning logic across databases
  • Partition metadata tracking: New table_partition_interval table persists partition configuration, decoupling interval choice from table structure
  • Improved bootstrapping: Separated partition creation logic into dedicated bootstrapper classes per database dialect
  • Enhanced testing: Comprehensive integration tests for both MySQL and PostgreSQL partition operations

🛠️ Changes Made

Database Schema:

  • Added partition_key column to alert_activations table (computed from activation_time)
  • Created table_partition_interval metadata table to track partition configuration per table
  • Updated primary key constraint to include partition_key for MySQL
  • Added migration c25e56faecce to handle schema updates and backfill existing data

Partitioning Logic:

  • Refactored PartitionInterval from partition.pypartition_interval.py with cleaner APIs
  • Renamed partititioner.pypartition_bootstrapper.py with dialect-specific implementations
  • Split partition creation logic into PartitionBootstrapperMySQL, PartitionBootstrapperPostgres, and PartitionBootstrapperSqlite
  • Introduced get_partition_key_value() and get_mysql_partition_key_sql() methods for consistent key generation
  • Removed database-specific partition expression queries in favor of stored metadata

Database Layer:

  • Added get_partition_interval_for_table() and set_partition_interval_for_table() to base DBInterface
  • Implemented dialect-specific create_partitions() and drop_partitions() methods
  • Improved SQLDB.__new__() to properly route SQLite connections
  • Added custom now() function with PostgreSQL-specific compilation for timestamp handling

Testing & CI:

  • Reorganized test structure with shared fixtures in tests/conftest.py
  • Added integration tests for partition operations on both MySQL and PostgreSQL
  • Updated CI workflow to run integration tests against both database backends
  • Improved coverage combination logic to handle multiple database test runs

Code Quality:

  • Better separation of concerns between bootstrapping and ongoing partition management
  • Reduced coupling between partition logic and database schema
  • More robust error handling and validation
  • Cleaner imports and reduced circular dependencies

✅ Checklist

  • I updated the documentation (if applicable)
  • I have tested the changes in this PR

🧪 Testing

Integration Tests:

  • test_partition_interval_yearweek_matches_mysql_yearweek_mode_1: Make sure the Python YEARWEEK partition key calculation matches MySQL's calaulcation
  • test_insert_populates_partition_key: Validates automatic partition key computation on insert
  • test_create_partitions_mysql/postgres: Tests partition creation with different intervals (DAY, MONTH, YEARWEEK)
  • test_drop_partitions_mysql/postgres: Validates retention-based partition cleanup
  • Tests run against real MySQL 8.0 and PostgreSQL 17 containers via pytest-mock-resources

Unit Tests:

  • Enhanced test_partitioner.py with parametrized tests for interval calculations
  • Tests for partition name generation, boundary computation, and interval math
  • Validation of environment variable parsing and metadata storage

Manual Testing:

  • Verified migration runs cleanly on both MySQL and PostgreSQL
  • Confirmed existing alert activations remain accessible after migration
  • Tested partition creation/deletion during normal API operations

CI Updates:

  • Integration tests now run against both mysql and postgres backends
  • Coverage reports correctly combine results from multi-database test runs
  • Migration tests validate against both database engines

🔗 References

  • Ticket link: ML-11553
  • Related: Original partitioning implementation in 650f0ce2da6f_add_alert_activation.py

🚨 Breaking Changes?

  • Yes (explain below)
  • No

Migration Required:

  • Existing deployments must run migration c25e56faecce which:
    • Adds partition_key column to alert_activations
    • Backfills partition keys for existing rows (MySQL only)
    • Updates primary key constraint
    • Creates table_partition_interval metadata table
  • PostgreSQL users: This is the first release with PostgreSQL partitioning support. Initial deployment will create partitioned tables automatically.
  • Environment variable: PARTITION_INTERVAL is now validated more strictly and must be one of: DAY, MONTH, YEARWEEK

🔍️ Additional Notes

Architecture Decisions:

  • Chose integer partition_key over expression-based partitioning for PostgreSQL compatibility and performance
  • Metadata table approach allows runtime interval changes in future (currently validated to prevent conflicts)
  • SQLite support included for local development, gracefully no-ops partition operations

Performance Considerations:

  • Partition key is computed once during insert via before_insert listener
  • No performance impact on existing queries (partition key included in PK)
  • Bulk backfill during migration uses single UPDATE statement

Future Work:

  • Allow setting partition intervals for other tables other than alert_activations

@liranbg liranbg mentioned this pull request Jan 13, 2026
10 tasks
@liranbg liranbg closed this Jan 13, 2026
liranbg added a commit that referenced this pull request Jan 13, 2026
### 📝 Description

Continuing the work done by @adamdelman @
#8347

---

This PR refactors and enhances the database partitioning system for
alert activations to improve cross-database compatibility, robustness,
and maintainability. The changes establish a cleaner architecture for
partition management while adding full PostgreSQL support alongside the
existing MySQL implementation.

Key improvements:

- Cross-database support: Added native PostgreSQL partitioning alongside
MySQL, with SQLite gracefully skipping partition operations
- Explicit partition keys: Introduced a dedicated partition_key integer
column computed from activation_time, enabling consistent partitioning
logic across databases
- Partition metadata tracking: New table_partition_interval table
persists partition configuration, decoupling interval choice from table
structure
- Improved bootstrapping: Separated partition creation logic into
dedicated bootstrapper classes per database dialect
- Enhanced testing: Comprehensive integration tests for both MySQL and
PostgreSQL partition operations


---

### 🛠️ Changes Made

- Integration tests now run against both mysql and postgres backends



---

### ✅ Checklist
- [ ] I updated the documentation (if applicable)
- [ ] I have tested the changes in this PR
- [ ] I confirmed whether my changes are covered by system tests
- [ ] If yes, I ran all relevant system tests and ensured they passed
before submitting this PR
- [ ] I updated existing system tests and/or added new ones if needed to
cover my changes
- [ ] If I introduced a deprecation:
  - [ ] I followed the [Deprecation Guidelines](./DEPRECATION.md)
  - [ ] I updated the relevant Jira ticket for documentation

---

### 🧪 Testing
<!-- - How it was tested (unit tests, manual, integration) -->  
<!-- - Any special cases covered. -->  

---

### 🔗 References
- Ticket link: https://iguazio.atlassian.net/browse/ML-11553
- Design docs links:
- External links:

---

### 🚨 Breaking Changes?

- [ ] Yes (explain below)
- [ ] No

<!-- If yes, describe what needs to be changed downstream: -->

---

### 🔍️ Additional Notes
<!-- Anything else reviewers should know (follow-up tasks, known issues,
affected areas etc.). -->
<!-- ### 📸 Screenshots / Logs -->

---------

Co-authored-by: adamdelman <flyn@flyn.cc>
Co-authored-by: Tal Haim <tal_haim@external.mckinsey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants