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

[columnar] Fix corruption possibilities for vacuum UDF #190

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

JerrySievert
Copy link
Contributor

this should fix #169.

there are two parts to the vacuum UDF:

  • combining stripes with deleted data
  • moving stripes into the new holes

bailing out in the combination process is not an issue, but bailing out during the reorganization is. if we exit cleanly from the signal handlers, instead of calling the signal after we feel we are safe, we do not have corruption. unfortunately, not calling the signal that we blocked is not an acceptable solution.

interestingly, finishing a single reorganization does not corrupt, even if the signal handlers are called. this PR sets up the vacuum UDF to do a singular reorganization, renames the UDF to columnar._vacuum_internal, and creates a new columnar.vacuum that recreates the original functionality by continually calling the columnar._vacuum_internal either until done, or we reach the optional count.

this does add additional time to the UDF.

note that this does increase the version of columnar, so we will need to upgrade the extension to fully fix the problem.

@JerrySievert JerrySievert self-assigned this Nov 1, 2023
@JerrySievert
Copy link
Contributor Author

I've also added elog statements at DEBUG3 which announce state transitions, so it should be fairly easy to test if you set your statement level to DEBUG3 and cancel/terminate after beginning reorganization.

Copy link
Contributor

@mkaruza mkaruza left a comment

Choose a reason for hiding this comment

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

lgtm

@JerrySievert JerrySievert merged commit 3876fa1 into main Nov 2, 2023
12 checks passed
@JerrySievert JerrySievert deleted the vacuum-corruption branch November 2, 2023 16:41
@muntdan
Copy link

muntdan commented Nov 3, 2023

note that this does increase the version of columnar, so we will need to upgrade the extension to fully fix the problem.

This means a new Tag/version will be published for this ?

@wuputah
Copy link
Member

wuputah commented Nov 3, 2023

every commit to main (well, any commit modifies code) has a build and tag. See:
https://github.com/hydradatabase/hydra/pkgs/container/hydra

I do plan on publishing a 1.0.2 release soon, but would be grateful for any testing you could do with the current build.

@muntdan
Copy link

muntdan commented Nov 4, 2023

So the “upgrade the extension" is included in the latest docker build?

@wuputah
Copy link
Member

wuputah commented Nov 4, 2023

This means you need to run ALTER EXTENSION columnar UPDATE; on all existing databases using columnar after upgrading to the new build.

(This is done automatically whenever we update the columnar build on Hydra Cloud, but not done automatically for Hydra's open source image.)

@wuputah wuputah mentioned this pull request Nov 6, 2023
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.

[Bug]: Stopping columnar vacuum can result in table corruption
4 participants