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

fix(meta): persist correct vnode count for table catalogs in sql backend #18759

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Sep 29, 2024

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

This is a progress towards #15900.

The table catalogs of internal tables are immediately persisted and broadcasted to other nodes when ids are allocated during the creation of a new streaming job. However, at that time we don't have scheduling info, i.e., the vnode_count of each fragment (then internal tables) hasn't been determined yet. As a result, we failed to persist the correct vnode_count in table catalogs.

In this PR, we fix this issue by updating that field in a separate step afterwards, together with other fields like fragment_id. Note that this only fixes the implementation for the SQL backend, since the etcd backend will be deprecated very soon (in #18727), making it less worthwhile to invest effort in it.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

@BugenZhao BugenZhao force-pushed the bz/var-vnode-system-tables branch from c2f007c to 7a83d1a Compare September 29, 2024 09:21
@BugenZhao BugenZhao force-pushed the bz/var-vnode-table-catalog-vnode-count branch from 61792a9 to eb7d2cf Compare September 29, 2024 09:21
@BugenZhao BugenZhao force-pushed the bz/var-vnode-system-tables branch from 7a83d1a to 07ec687 Compare September 30, 2024 03:05
@BugenZhao BugenZhao force-pushed the bz/var-vnode-table-catalog-vnode-count branch from eb7d2cf to beba88d Compare September 30, 2024 03:05
Base automatically changed from bz/var-vnode-system-tables to main September 30, 2024 03:34
@graphite-app graphite-app bot requested a review from a team September 30, 2024 03:35
@BugenZhao BugenZhao force-pushed the bz/var-vnode-table-catalog-vnode-count branch from beba88d to 23a1d96 Compare October 8, 2024 05:40
@BugenZhao BugenZhao changed the title fix(meta): persist correct vnode count for catalogs fix(meta): persist correct vnode count for catalogs in sql backend Oct 8, 2024
@BugenZhao BugenZhao changed the title fix(meta): persist correct vnode count for catalogs in sql backend fix(meta): persist correct vnode count for table catalogs in sql backend Oct 8, 2024
@BugenZhao BugenZhao requested a review from yezizp2012 October 8, 2024 06:11
@BugenZhao BugenZhao added this pull request to the merge queue Oct 8, 2024
Merged via the queue into main with commit c174d91 Oct 8, 2024
39 of 40 checks passed
@BugenZhao BugenZhao deleted the bz/var-vnode-table-catalog-vnode-count branch October 8, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants