Page MenuHomePhabricator

Add table to track migration of secondary indexes on the event_log table
ClosedPublic

Authored by prha on Oct 16 2020, 2:16 PM.

Details

Summary

For SQL-event log implementations, adds a check for event_log migration status.
The instance implementations fronts those migration status queries with an in-memory cache.

This does mean that dagit would need to be restarted to pick up any migration change, but
seems worth it.

Name change suggestions welcome.

Test Plan

BK

Diff Detail

Repository
R1 dagster
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

prha created this revision.

black

prha requested review of this revision.Oct 16 2020, 2:39 PM
prha added reviewers: alangenfeld, max.

update, share secondary_indexes table across run, event_log storage.

remove hard-coded version check

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py
61–62

do we need the schema migration? I was under the impression this took care of it for us?

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py
61–62

oh. We might not need it... I just created the migrations to add the tables out of habit.

python_modules/dagster/dagster/core/storage/event_log/sqlite/consolidated_sqlite_event_log.py
108–116

these should be defined on the event log base class to be safe

also, these names are a bit odd to me is_index_enabled / set_index_enabled , has_secondary_index / enable_secondary_index is where my head goes

πŸ™

python_modules/dagster/dagster/core/storage/event_log/sqlite/consolidated_sqlite_event_log.py
115–117

add a test around this cache clearing behavior?

This revision is now accepted and ready to land.Oct 20 2020, 10:53 PM
prha retitled this revision from RFC: Add table to track migration of secondary indexes on the event_log table to Add table to track migration of secondary indexes on the event_log table.Oct 21 2020, 2:14 PM