Skip to content
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

[TECH DEBT]: Inefficient use of TableMigrationStatusRefresher.index() #2730

Closed
asnare opened this issue Sep 24, 2024 · 0 comments · Fixed by #3200
Closed

[TECH DEBT]: Inefficient use of TableMigrationStatusRefresher.index() #2730

asnare opened this issue Sep 24, 2024 · 0 comments · Fixed by #3200
Assignees
Labels
feat/migration-index mapping of databases to catalog or potentially other databases feat/migration-progress Issues related to the migration progress workflow migrate/code Abstract Syntax Trees and other dark magic tech debt chores and design flaws

Comments

@asnare
Copy link
Contributor

asnare commented Sep 24, 2024

Problem statement

The TableMigrationStatusRefresher.index() method is used to load the migration state of all tables and views in the inventory. The fast-path is that this loads the migration state from the inventory table (select * from migration_status).

We have a lot of code-paths that invoke this function to obtain the index, especially in the migration-related code, not realising that it's relatively expensive. One particular case is: TablesMigrator.get_remaining_tables() which loads this index (and discards it) within a loop over every table in the inventory.

Proposed Solution

At the very least TablesMigrator.get_remaining_tables() should be refactored to only build the index once.

I haven't some a full survey of usage, but there are probably many other places where refactoring is needed to avoid redundant loading of the index.

@asnare asnare added enhancement New feature or request needs-triage labels Sep 24, 2024
@asnare asnare added tech debt chores and design flaws and removed enhancement New feature or request labels Sep 24, 2024
@nfx nfx added feat/migration-index mapping of databases to catalog or potentially other databases migrate/code Abstract Syntax Trees and other dark magic feat/migration-progress Issues related to the migration progress workflow and removed needs-triage labels Oct 9, 2024
@nfx nfx moved this from Triage to Quarter Backlog in UCX (roadmap) Oct 9, 2024
@asnare asnare self-assigned this Nov 4, 2024
@nfx nfx closed this as completed in #3200 Nov 11, 2024
@nfx nfx closed this as completed in 403e4fd Nov 11, 2024
@github-project-automation github-project-automation bot moved this from Quarter Backlog to Archive in UCX (roadmap) Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat/migration-index mapping of databases to catalog or potentially other databases feat/migration-progress Issues related to the migration progress workflow migrate/code Abstract Syntax Trees and other dark magic tech debt chores and design flaws
Projects
Status: Archive
2 participants