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

Disable the LIR instrospection sources again #30650

Merged

Conversation

teskje
Copy link
Contributor

@teskje teskje commented Nov 28, 2024

This PR again disables the newly added introspection sources mz_compute_dataflow_global_ids_per_worker and
mz_compute_lir_mapping_per_worker, because Mz's 0dt mechanism is currently not prepared to handle new introspection sources. A fix for the 0dt mechanism was merged (#30603) but has to be reverted because it is suspected of causing panics: https://github.com/MaterializeInc/database-issues/issues/8799

Motivation

  • This PR fixes a recognized bug.

Tips for reviewer

I just reused my original commit instead of reverting the revert (#30620). I hope that makes no difference.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

This commit disables the newly added introspection sources
`mz_compute_dataflow_global_ids_per_worker` and
`mz_compute_lir_mapping_per_worker`, because Mz's 0dt mechanism is
currently not prepared to handle new introspection sources.

"Disabling" here means removing the registration of the new `Log`
builtins and the `View`s depending on them. The intention is to keep the
diff as small as possible, to make this change less risky.
@teskje teskje requested review from a team as code owners November 28, 2024 17:29
@teskje teskje requested review from ParkMyCar and ggevay November 28, 2024 17:29
Copy link
Contributor

@def- def- left a comment

Choose a reason for hiding this comment

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

Oh no...

@benesch
Copy link
Member

benesch commented Nov 28, 2024

I'm going to merge this but not #30649 on a hunch.

@benesch benesch merged commit a3aafa6 into MaterializeInc:main Nov 28, 2024
86 checks passed
jkosh44 added a commit to jkosh44/materialize that referenced this pull request Dec 2, 2024
…ew-introspection-sources"

This reverts commit a3aafa6, reversing
changes made to 33af11a.
jkosh44 added a commit to jkosh44/materialize that referenced this pull request Dec 3, 2024
…ew-introspection-sources"

This reverts commit a3aafa6, reversing
changes made to 33af11a.
jkosh44 added a commit that referenced this pull request Dec 3, 2024
#30692)

…on-sources"

This reverts commit a3aafa6, reversing
changes made to 33af11a.
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.

3 participants