Skip to content

Fix llmeta schema change object leaks - #6155

Open
dorinhogea wants to merge 1 commit into
bloomberg:mainfrom
dorinhogea:sc-llmeta-leaks
Open

Fix llmeta schema change object leaks#6155
dorinhogea wants to merge 1 commit into
bloomberg:mainfrom
dorinhogea:sc-llmeta-leaks

Conversation

@dorinhogea

Copy link
Copy Markdown
Contributor

osql_finalize_scs deleted the sc_list keyed by the schema change uuid, set only for TRANLEVEL_SOSQL; key it by the osql session uuid instead.

Only kinds running through do_ddl() reach mark_schemachange_over_tran(), so stop persisting an sc seed for the others (sp, trigger, lua func) -- nothing ever removed it. Resumed sc lists no longer mark those kinds as resuming, as they have no persisted state to pick up.

OSQL_FLAGS_SCDONE is set when a schema change is dispatched, before a downgrade can be seen by its async conversion, and was never cleared, so the abort path deleted the sc_list the new master needed to resume from.

A failed multi-table resume dropped its ireq, orphaning the schema changes that had started and stranding the sc_list, and gave up on the remaining sc lists.

re: 185631797

osql_finalize_scs deleted the sc_list keyed by the schema change uuid,
set only for TRANLEVEL_SOSQL; key it by the osql session uuid instead.

Only kinds running through do_ddl() reach mark_schemachange_over_tran(),
so stop persisting an sc seed for the others (sp, trigger, lua func) --
nothing ever removed it.  Resumed sc lists no longer mark those kinds as
resuming, as they have no persisted state to pick up.

OSQL_FLAGS_SCDONE is set when a schema change is dispatched, before a
downgrade can be seen by its async conversion, and was never cleared, so
the abort path deleted the sc_list the new master needed to resume from.

A failed multi-table resume dropped its ireq, orphaning the schema
changes that had started and stranding the sc_list, and gave up on the
remaining sc lists.

Signed-off-by: Dorin Hogea <[email protected]>

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
sc_downgrade [timeout] **quarantined**
truncatesc_offline_generated [timeout] **quarantined**

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
sc_truncate [db unavailable at finish]
consumer_non_atomic_default_consumer_generated **quarantined**
unifiedcancel **quarantined**
sc_downgrade [timeout] **quarantined**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants