Details

Reviewers

alangenfeld
• max

Commits

R1:3e602b0902d6: Add table to track migration of secondary indexes on the event_log table

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

Diff Detail

Repository

R1 dagster

Lint

Lint Passed

Unit

No Test Coverage

Event Timeline

black

Harbormaster failed remote builds in B19677: Diff 23889!Oct 16 2020, 2:34 PM

Harbormaster completed remote builds in B19678: Diff 23890.Oct 16 2020, 2:39 PM

• prha requested review of this revision.Oct 16 2020, 2:39 PM

• prha edited the summary of this revision. (Show Details)Oct 16 2020, 2:59 PM

• prha added reviewers: alangenfeld, • max.

• prha mentioned this in D4764: add summary table for asset keys.Oct 16 2020, 4:22 PM

update, share secondary_indexes table across run, event_log storage.

Harbormaster failed remote builds in B19755: Diff 23976!Oct 19 2020, 3:33 PM

remove hard-coded version check

Harbormaster failed remote builds in B19758: Diff 23979!Oct 19 2020, 4:36 PM

update

Harbormaster completed remote builds in B19765: Diff 23986.Oct 19 2020, 5:36 PM

alangenfeld added inline comments.Oct 19 2020, 6:43 PM

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py
57–58	do we need the schema migration? I was under the impression this took care of it for us?

• prha added inline comments.Oct 19 2020, 7:15 PM

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py
57–58	oh. We might not need it... I just created the migrations to add the tables out of habit.

alangenfeld added inline comments.Oct 19 2020, 7:39 PM

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

comments

Harbormaster completed remote builds in B19808: Diff 24032.Oct 19 2020, 10:18 PM

🙏

python_modules/dagster/dagster/core/storage/event_log/sqlite/consolidated_sqlite_event_log.py
116–118	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

tests, rebase

Harbormaster completed remote builds in B19927: Diff 24179.Oct 21 2020, 2:40 PM

Closed by commit R1:3e602b0902d6: Add table to track migration of secondary indexes on the event_log table (authored by • prha). · Explain WhyOct 21 2020, 2:44 PM

This revision was automatically updated to reflect the committed changes.

• prha added a commit: R1:3e602b0902d6: Add table to track migration of secondary indexes on the event_log table.

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 23890

python_modules/dagster/dagster/core/storage/event_log/schema.py

python_modules/dagster/dagster/core/storage/event_log/sql_event_log.py

python_modules/dagster/dagster/core/storage/event_log/sqlite/alembic/versions/ef06c197ffdc_create_secondary_index_table.py

python_modules/dagster/dagster/core/storage/event_log/sqlite/consolidated_sqlite_event_log.py

python_modules/dagster/dagster/core/storage/event_log/sqlite/sqlite_event_log.py

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs.db

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs.db-shm

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs.db-wal

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/5b5bf65b-32b8-4b8f-a672-37358ca47907.db

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/5b5bf65b-32b8-4b8f-a672-37358ca47907.db-shm

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/5b5bf65b-32b8-4b8f-a672-37358ca47907.db-wal

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/7333670e-3b10-4253-abdb-3dfdb7fbff71.db

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/7333670e-3b10-4253-abdb-3dfdb7fbff71.db-shm

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/7333670e-3b10-4253-abdb-3dfdb7fbff71.db-wal

python_modules/dagster/dagster_tests/general_tests/compat_tests/test_back_compat.py

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/alembic/versions/6c0d40c3d559_create_secondary_index_table.py

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py

python_modules/libraries/dagster-postgres/dagster_postgres/run_storage/alembic/versions/6c0d40c3d559_create_secondary_index_table.py

python_modules/libraries/dagster-postgres/dagster_postgres/schedule_storage/alembic/versions/6c0d40c3d559_create_secondary_index_table.py

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 23890

python_modules/dagster/dagster/core/storage/event_log/schema.py

python_modules/dagster/dagster/core/storage/event_log/sql_event_log.py

python_modules/dagster/dagster/core/storage/event_log/sqlite/alembic/versions/ef06c197ffdc_create_secondary_index_table.py

python_modules/dagster/dagster/core/storage/event_log/sqlite/consolidated_sqlite_event_log.py

python_modules/dagster/dagster/core/storage/event_log/sqlite/sqlite_event_log.py

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs.db

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs.db-shm

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs.db-wal

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/5b5bf65b-32b8-4b8f-a672-37358ca47907.db

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/5b5bf65b-32b8-4b8f-a672-37358ca47907.db-shm

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/5b5bf65b-32b8-4b8f-a672-37358ca47907.db-wal

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/7333670e-3b10-4253-abdb-3dfdb7fbff71.db

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/7333670e-3b10-4253-abdb-3dfdb7fbff71.db-shm

python_modules/dagster/dagster_tests/general_tests/compat_tests/snapshot_0_9_16_pre_secondary_index_migration/sqlite/history/runs/7333670e-3b10-4253-abdb-3dfdb7fbff71.db-wal

python_modules/dagster/dagster_tests/general_tests/compat_tests/test_back_compat.py

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/alembic/versions/6c0d40c3d559_create_secondary_index_table.py

python_modules/libraries/dagster-postgres/dagster_postgres/event_log/event_log.py

python_modules/libraries/dagster-postgres/dagster_postgres/run_storage/alembic/versions/6c0d40c3d559_create_secondary_index_table.py

python_modules/libraries/dagster-postgres/dagster_postgres/schedule_storage/alembic/versions/6c0d40c3d559_create_secondary_index_table.py

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

Revision Contents
Changeset List