Page MenuHomePhabricator

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

Authored by prha on Fri, Oct 16, 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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

prha created this revision.

black

prha requested review of this revision.Fri, Oct 16, 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
60โ€“61

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
60โ€“61

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
107โ€“115

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.Tue, Oct 20, 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.Wed, Oct 21, 2:14 PM