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

chore: events: Prevent DDL re-execution during event index schema migrations #11717

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

masih
Copy link
Member

@masih masih commented Mar 13, 2024

Related Issues

Proposed Changes

TL;DR: no net functionality change but a more conservative DDL execution for existing event index schema migrations.

This enhancement optimizes the schema migration process for the event index by preventing the redundant execution of Data Definition Language (DDL) statements that define the event schema. Traditionally, these DDL statements were grouped into a single slice, reflecting the most current version of the event index schema. With each migration, this slice was updated to the latest schema iteration, executing all statements in bulk. Initially, this method sufficed as migrations were focused on adding indices to existing table columns.

However, as the database schema evolves to meet new requirements, such as the forthcoming migrations that involve changes to table schemas (notably, indexing events by emitter actor ID instead of addresses), the prior approach of bulk execution of DDL statements becomes unsuitable: it will no longer be safe to repeatedly execute DDL statements in previous migrations, because the upcoming one changes event table column structure. To address this issue, the work here has isolated the event index schema migrations on a per-version basis. This adjustment ensures that only the necessary DDL statements are executed during each migration, avoiding the inefficiencies and potential errors associated with redundant executions.

Additional Info

The work here:

  • should also minimize the refactoring required for future migrations, in favour of smaller more manageable PRs.
  • makes changes to the existing code to respect the provided context during migration execution.

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@masih masih changed the title chore: events:Prevent DDL re-execution during event index schema migrations chore: events: Prevent DDL re-execution during event index schema migrations Mar 13, 2024
@masih masih force-pushed the masih/events-index-schema-isloate-ddls branch from b578e83 to 80d08b9 Compare March 13, 2024 18:40
@masih masih marked this pull request as ready for review March 13, 2024 18:40
@masih masih requested review from fridrik01 and removed request for aarshkshah1992, rjan90 and jennijuju March 13, 2024 18:40
This enhancement optimizes the schema migration process for the event
index by preventing the redundant execution of Data Definition Language
(DDL) statements that define the event schema. Traditionally, these DDL
statements were grouped into a single slice, reflecting the most current
version of the event index schema. With each migration, this slice was
updated to the latest schema iteration, executing all statements in
bulk. Initially, this method sufficed as migrations were focused on
adding indices to existing table columns.

However, as the database schema evolves to meet new requirements, such
as the forthcoming migrations that involve changes to table schemas
(notably, indexing events by emitter actor ID instead of addresses),
the prior approach of bulk execution of DDL statements becomes
unsuitable: it will no longer be safe to repeatedly execute DDL
statements in previous migrations, because the upcoming one changes
`event` table column structure. To address this issue, the work here has
isolated the event index schema migrations on a per-version basis. This
adjustment ensures that only the necessary DDL statements are executed
during each migration, avoiding the inefficiencies and potential errors
associated with redundant executions.

The work here should also minimize the refactoring required for future
migrations, facilitating a smoother introduction of significant schema
updates.
@masih masih force-pushed the masih/events-index-schema-isloate-ddls branch from 80d08b9 to e53a933 Compare March 14, 2024 08:45
@masih masih requested review from aarshkshah1992 and removed request for Stebalien, snadrus, magik6k, ZenGround0 and arajasek March 14, 2024 10:06
@Stebalien Stebalien requested a review from rvagg March 14, 2024 20:52
@masih masih enabled auto-merge (rebase) March 15, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

3 participants