fix(meta): persist internal tables of `CREATE TABLE` #13039

kwannoel · 2023-10-25T00:07:14Z

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

What's changed and what's your intention?

It follows the same path as materialized view. Its TABLEs have to be persisted as well. Otherwise if CN down, and meta tries to cancel the CREATE TABLE ddl, it will encounter errors when trying to delete the internal tables for state cleanup.

Separately make the update of tables transactional, so that either clean_dirty_table or cancel_create_table_procedure will remove tables from meta store, but not both.

Finally, also remove assertions for create table with source, because if recovery happens, before stream job is cancelled, the in memory creation trackers could already have been cleared.

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

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.

This reverts commit 2e2e7ce.

kwannoel · 2023-10-25T04:29:01Z

Main cron tests now pass, except for the flaky backfill test. That is failing due to startup and teardown of cluster.

kwannoel · 2023-10-25T04:44:45Z

chenzl25 · 2023-10-25T07:08:55Z

src/meta/src/manager/catalog/mod.rs

-                && !database_core.tables.contains_key(&table.id)
-                && database_core.has_in_progress_creation(&source_key)
-                && database_core.has_in_progress_creation(&table_key),
+                && !database_core.tables.contains_key(&table.id),


Could you please shed some light on this removal?

When clean_dirty_tables, we will clean up all the in memory tracking state as well.

As a result, these assertions could fail.

Reason being, we could have the following sequence of events:

Stream job fails, triggering recovery.

Recovery calls clean dirty tables, causing the creation trackers to be cleared.

The failing stream job calls cleanup via this function (or other cancel_xxx function, depending on stream job type).

Since 2 could already clean up the trackers, we should not assert it here.

Can we fully deprecate these in-memory tracking status? Or it's only feasible until we support background creation for other type of streaming jobs. The cleaning part is getting confusing, hope it can be more clear upon new meta backend. 🥵

After sink and index are persisted I will deprecate it

yezizp2012 · 2023-10-25T08:48:25Z

src/meta/src/manager/catalog/mod.rs

-                && !database_core.tables.contains_key(&table.id)
-                && database_core.has_in_progress_creation(&source_key)
-                && database_core.has_in_progress_creation(&table_key),
+                && !database_core.tables.contains_key(&table.id),


Can we fully deprecate these in-memory tracking status? Or it's only feasible until we support background creation for other type of streaming jobs. The cleaning part is getting confusing, hope it can be more clear upon new meta backend. 🥵

commit b52a004 Author: xxchan <[email protected]> Date: Thu Oct 26 14:25:18 2023 +0800 update arrow-ipc commit e94feeb Author: xxchan <[email protected]> Date: Thu Oct 26 06:21:34 2023 +0000 Fix "cargo-hakari" commit 08a5601 Merge: 56e6fc4 942e99d Author: xxchan <[email protected]> Date: Thu Oct 26 14:19:34 2023 +0800 Merge branch 'main' into xxchan/wasm-udf commit 942e99d Author: Yufan Song <[email protected]> Date: Wed Oct 25 22:10:31 2023 -0700 fix(nats-connector): change stream into optional string, add replace stream name logic (#13024) commit 90fb4a3 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu Oct 26 04:25:11 2023 +0000 chore(deps): Bump comfy-table from 7.0.1 to 7.1.0 (#13049) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit b724be7 Author: jinser <[email protected]> Date: Thu Oct 26 00:26:15 2023 +0800 feat: add `comment on` clause support (#12849) Co-authored-by: Richard Chien <[email protected]> Co-authored-by: August <[email protected]> commit 7f791d6 Author: August <[email protected]> Date: Wed Oct 25 20:29:16 2023 +0800 feat: move model_v2 and model_migration into a separate crates (#13058) commit 7f82929 Author: Noel Kwan <[email protected]> Date: Wed Oct 25 16:57:45 2023 +0800 fix(meta): persist internal tables of `CREATE TABLE` (#13039) commit 09a67ab Author: Noel Kwan <[email protected]> Date: Wed Oct 25 16:49:08 2023 +0800 fix: `WAIT` should return error if timeout (#13045) commit e48547d Author: Runji Wang <[email protected]> Date: Wed Oct 25 16:41:16 2023 +0800 refactor(type): switch jsonb to flat representation (#12952) Signed-off-by: Runji Wang <[email protected]> commit 56e6fc4 Author: xxchan <[email protected]> Date: Wed Oct 25 15:33:36 2023 +0800 fix merge issue commit c644361 Merge: fcd6992 2d428b1 Author: xxchan <[email protected]> Date: Wed Oct 25 15:23:44 2023 +0800 Merge remote-tracking branch 'origin/main' into xxchan/wasm-udf commit fcd6992 Author: xxchan <[email protected]> Date: Wed Oct 25 14:28:53 2023 +0800 fix s3 stuck commit 21e9740 Author: xxchan <[email protected]> Date: Wed Oct 25 12:47:24 2023 +0800 Revert "fix s3 stuck (why?)" This reverts commit f19a6b4. commit f19a6b4 Author: xxchan <[email protected]> Date: Wed Sep 13 14:32:28 2023 +0800 fix s3 stuck (why?) commit 019f309 Author: xxchan <[email protected]> Date: Tue Sep 12 15:29:52 2023 +0800 ON_ERROR_STOP=1 commit 6e4ee3c Author: xxchan <[email protected]> Date: Tue Sep 12 15:09:58 2023 +0800 generate-config commit b63a1c3 Merge: 2b0cc96 53611bf Author: xxchan <[email protected]> Date: Tue Sep 12 14:53:10 2023 +0800 Merge remote-tracking branch 'origin/main' into xxchan/wasm-udf commit 2b0cc96 Author: xxchan <[email protected]> Date: Sat Sep 9 23:49:43 2023 +0800 fix conflicts commit 6b13fe3 Author: xxchan <[email protected]> Date: Sat Sep 9 23:35:50 2023 +0800 update system param default commit a273943 Merge: cc34bfe f649aa6 Author: xxchan <[email protected]> Date: Sat Sep 9 23:33:38 2023 +0800 Merge remote-tracking branch 'origin/main' into xxchan/wasm-udf commit cc34bfe Author: xxchan <[email protected]> Date: Tue Aug 1 17:47:42 2023 +0200 use count_char as the example commit f913f63 Merge: 53bf8e0 2637dbd Author: xxchan <[email protected]> Date: Tue Aug 1 17:22:13 2023 +0200 Merge branch 'main' into xxchan/wasm-udf commit 53bf8e0 Author: xxchan <[email protected]> Date: Mon Jul 31 14:20:07 2023 +0200 minor update commit 70cee42 Author: xxchan <[email protected]> Date: Mon Jul 17 14:53:29 2023 +0200 fix arrow_schema into -> try_into commit a7d172d Author: xxchan <[email protected]> Date: Fri Jul 14 16:31:20 2023 +0200 buf format commit 43a3290 Author: xxchan <[email protected]> Date: Thu Jul 13 23:04:16 2023 +0200 add tinygo example & turn on wasi support commit 61a4998 Author: xxchan <[email protected]> Date: Wed Jul 12 11:40:56 2023 +0200 cleanup commit 165d4d9 Author: xxchan <[email protected]> Date: Wed Jul 12 11:02:44 2023 +0200 use object store to store wasm commit 88979e4 Author: xxchan <[email protected]> Date: Tue Jul 11 15:32:52 2023 +0200 add wasm_storage_url system param commit a897320 Author: xxchan <[email protected]> Date: Thu Jul 6 20:04:45 2023 +0200 Load compiled wasm module in expr 🚀🚀🚀 commit 63b3523 Author: xxchan <[email protected]> Date: Sun Jul 2 19:27:22 2023 +0200 it works (although very slow)

ensure transactionality of cancel create table procedure

03f8d2c

kwannoel added ci/run-build-simulation ci/skip-ci ci/run-recovery-test-deterministic-simulation labels Oct 25, 2023

github-actions bot added the type/fix Bug fix label Oct 25, 2023

increase the kill rate to match

2e2e7ce

kwannoel changed the title ~~fix(meta): ensure transactionality of cancel create table procedure~~ fix(meta): ensure transactionality when cleaning tables Oct 25, 2023

kwannoel added the ci/run-main-cron label Oct 25, 2023

make clean dirty tables transactional

b2497f9

kwannoel force-pushed the kwannoel/fix-recovery branch from cecbf84 to b2497f9 Compare October 25, 2023 00:21

kwannoel removed the ci/run-main-cron label Oct 25, 2023

kwannoel added 3 commits October 25, 2023 08:43

add more tracing

cf08f63

pass internal tables

7cfffda

Revert "increase the kill rate to match"

f7c9863

This reverts commit 2e2e7ce.

kwannoel added ci/run-main-cron and removed ci/skip-ci labels Oct 25, 2023

kwannoel marked this pull request as ready for review October 25, 2023 01:40

kwannoel requested review from yezizp2012, BugenZhao and chenzl25 October 25, 2023 01:41

kwannoel changed the title ~~fix(meta): ensure transactionality when cleaning tables~~ fix(meta): persist internal tables of CREATE TABLE Oct 25, 2023

kwannoel added 2 commits October 25, 2023 09:46

fmt

4b3ca43

clean dirty tables could already clear the in progress creation

b3c69e0

chenzl25 reviewed Oct 25, 2023

View reviewed changes

yezizp2012 approved these changes Oct 25, 2023

View reviewed changes

kwannoel added this pull request to the merge queue Oct 25, 2023

Merged via the queue into main with commit 7f82929 Oct 25, 2023
7 of 8 checks passed

kwannoel deleted the kwannoel/fix-recovery branch October 25, 2023 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(meta): persist internal tables of `CREATE TABLE` #13039

fix(meta): persist internal tables of `CREATE TABLE` #13039

kwannoel commented Oct 25, 2023 •

edited

Loading

kwannoel commented Oct 25, 2023 •

edited

Loading

kwannoel commented Oct 25, 2023

chenzl25 Oct 25, 2023

kwannoel Oct 25, 2023

kwannoel Oct 25, 2023 •

edited

Loading

yezizp2012 Oct 25, 2023

kwannoel Oct 25, 2023

yezizp2012 Oct 25, 2023

fix(meta): persist internal tables of CREATE TABLE #13039

fix(meta): persist internal tables of CREATE TABLE #13039

Conversation

kwannoel commented Oct 25, 2023 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

kwannoel commented Oct 25, 2023 • edited Loading

kwannoel commented Oct 25, 2023

chenzl25 Oct 25, 2023

Choose a reason for hiding this comment

kwannoel Oct 25, 2023

Choose a reason for hiding this comment

kwannoel Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

yezizp2012 Oct 25, 2023

Choose a reason for hiding this comment

kwannoel Oct 25, 2023

Choose a reason for hiding this comment

yezizp2012 Oct 25, 2023

Choose a reason for hiding this comment

fix(meta): persist internal tables of `CREATE TABLE` #13039

fix(meta): persist internal tables of `CREATE TABLE` #13039

kwannoel commented Oct 25, 2023 •

edited

Loading

kwannoel commented Oct 25, 2023 •

edited

Loading

kwannoel Oct 25, 2023 •

edited

Loading