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

coord: enable select sys table persistence behind a flag (default off) #7696

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

danhhz
Copy link
Contributor

@danhhz danhhz commented Aug 4, 2021

Rename --persistent_tables to --persistent_user_tables and add
--persistent_system_tables. The latter currently defaults to off and
when on, only opts in the system tables that have self-elected to be
persisted (mz_metrics and mz_metrics_histogram). Once we've stabilized
persistence, we'll default --persistent_system_tables to on, but keep
the flag as a CYA.

Along the way, fix a bug where turning on persisted system tables also
created an unnecessary persistent stream for all system views.

Also improve the persistent stream naming, which would have helped debug
what was going on with this view bug.

@danhhz danhhz requested a review from ruchirK August 4, 2021 20:51
@danhhz
Copy link
Contributor Author

danhhz commented Aug 4, 2021

cc @benesch in case he has any opinions on this

@benesch
Copy link
Member

benesch commented Aug 4, 2021

No opinions! Seems good!

@danhhz danhhz mentioned this pull request Aug 4, 2021
39 tasks
@danhhz
Copy link
Contributor Author

danhhz commented Aug 6, 2021

Hi! Friendly ping on this @ruchirK :)

Copy link
Contributor

@ruchirK ruchirK left a comment

Choose a reason for hiding this comment

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

LGTM! Just to double check -- if a user runs with --persistent-user-tables --experimental creates, a table and then inserts some rows, then restarts with just --experimental they will observe the table but not the previously persisted rows from the last run. Then if they insert some rows those will not be persisted. Then if they run mz again with --persistent-user-tables --experimental they will observe the table with only the rows inserted in the very first run.

If that understanding is correct then this is all great!

@danhhz
Copy link
Contributor Author

danhhz commented Aug 6, 2021

TFTR!

Just ran through your script locally and verified that it does indeed work exactly like that. At some point, we'll want to figure out exactly how all this should work for existing tables, new tables (in existing vs new cluster), etc and write tests since it's easy to mess this stuff up. Feels a bit early for that still IMO, but it's coming up soon

Btw, it looks like SELECT is hanging on persisted user tables at HEAD. I suspect either #7686 or #7659 but we should get that fixed ASAP

Rename --persistent_tables to --persistent_user_tables and add
--persistent_system_tables. The latter currently defaults to off and
when on, only opts in the system tables that have self-elected to be
persisted (mz_metrics and mz_metrics_histogram). Once we've stabilized
persistence, we'll default --persistent_system_tables to on, but keep
the flag as a CYA.

Along the way, fix a bug where turning on persisted system tables also
created an unnecessary persistent stream for all system views.

Also improve the persistent stream naming, which would have helped debug
what was going on with this view bug.
@danhhz danhhz merged commit b129a67 into MaterializeInc:main Aug 6, 2021
@danhhz danhhz deleted the persist_systables branch September 29, 2021 18:40
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.

None yet

3 participants