feat(frontend): support execute insert in local mode #8208

ZENOTME · 2023-02-27T13:21:42Z

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

What's changed and what's your intention?

related issue: #7684

support to execute insert without using select query in local mode, such as: insert into t values (1)

For other dml like: insert-select, delete, update, their plan has more than two stage so that we can't execute them in local mode.

Checklist For Contributors

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).
I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future. (Please refer to the issue)
All checks passed in ./risedev check (or alias, ./risedev c)

Checklist For Reviewers

I have requested macro/micro-benchmarks as this PR can affect performance substantially, and the results are shown.

Documentation

Click here for Documentation

Types of user-facing changes

SQL commands, functions, and operators

Release note

In past the insert will be executed in distributed mode, but now the insert without using select query will executed in local mode.

codecov · 2023-03-01T09:24:49Z

Codecov Report

Merging #8208 (45b25a0) into main (1ad23ba) will decrease coverage by 0.02%.
The diff coverage is 62.28%.

@@            Coverage Diff             @@
##             main    #8208      +/-   ##
==========================================
- Coverage   71.65%   71.63%   -0.02%     
==========================================
  Files        1131     1131              
  Lines      184150   184230      +80     
==========================================
+ Hits       131948   131978      +30     
- Misses      52202    52252      +50

Flag	Coverage Δ
rust	`71.63% <62.28%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/frontend/src/scheduler/local.rs	`0.00% <0.00%> (ø)`
src/frontend/src/handler/query.rs	`17.74% <45.00%> (+1.78%)`	⬆️
src/frontend/src/optimizer/mod.rs	`90.88% <100.00%> (+0.80%)`	⬆️
src/utils/pgwire/src/pg_response.rs	`67.18% <0.00%> (-3.13%)`	⬇️
...torage/src/hummock/local_version/pinned_version.rs	`88.75% <0.00%> (-0.63%)`	⬇️
src/meta/src/hummock/mock_hummock_meta_client.rs	`65.46% <0.00%> (-0.52%)`	⬇️
src/object_store/src/object/mem.rs	`86.87% <0.00%> (-0.39%)`	⬇️
src/storage/src/hummock/compactor/iterator.rs	`96.40% <0.00%> (-0.28%)`	⬇️
src/storage/src/hummock/sstable_store.rs	`64.77% <0.00%> (-0.16%)`	⬇️
src/stream/src/executor/aggregation/minput.rs	`96.25% <0.00%> (-0.11%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

lmatz · 2023-03-01T13:48:05Z

Do you want to test insert in both local mode and distributed mode in the nightly performance test?
https://buildkite.com/risingwave-test/sysbench

ZENOTME · 2023-03-02T00:21:37Z

Do you want to test insert in both local mode and distributed mode in the nightly performance test? https://buildkite.com/risingwave-test/sysbench

Yes, how I test that

lmatz · 2023-03-02T04:35:23Z

Do you want to test insert in both local mode and distributed mode in the nightly performance test? https://buildkite.com/risingwave-test/sysbench

Yes, how I test that

QA team will add local mode into the existing performance test pipeline

src/frontend/src/handler/query.rs

kwannoel · 2023-03-02T05:56:12Z

src/frontend/src/scheduler/local.rs

+                Ok(vec![self.front_env.worker_node_manager().next_random()?])
+            }
+        } else {
+            Ok(self.front_env.worker_node_manager().list_worker_nodes())


Hmm should we list all worker nodes here?
What if parallelism = 3, and worker nodes = 5, does that mean we choose to schedule 5 workers?
Should it match exactly? i.e. choose N workers for N parallelism.

I think it should be workers with target table only.

We have processed the case that the stage have scan node before, so in this case the stage don't have the scan node, I think we just need to randomlly select N parallelism worker?🤔

(Previous process is list worker node)

risingwave/src/frontend/src/scheduler/local.rs

Line 325 in 1ad23ba

self.front_env.worker_node_manager().list_worker_nodes()

kwannoel · 2023-03-02T06:01:23Z

src/frontend/src/scheduler/local.rs

+                    let worker_node = {
+                        let parallel_unit_ids = vnode_mapping.iter_unique().collect_vec();
+                        let candidates = self.front_env
+                            .worker_node_manager()
+                            .get_workers_by_parallel_unit_ids(&parallel_unit_ids)?;
+                        candidates.choose(&mut rand::thread_rng()).unwrap().clone()
+                    };


Why do we need get_workers_by_parallel_unit_ids here? Didn't use this before, so not too familiar.
In the case of Insert why is this needed?

The insert executor will send the insert data to the reader registered by the dml executor.

risingwave/src/batch/src/executor/insert.rs

Line 134 in 1ad23ba

.write_chunk(self.table_id, self.table_version_id, stream_chunk)

risingwave/src/stream/src/executor/dml.rs

Line 94 in 1ad23ba

let batch_reader = batch_reader.stream_reader().into_stream();

Hence to access the reader, the insert executor need to schedule the same worker node of the dml executor. So get_workers_by_parallel_unit_ids is to get the worker node where dml executor stay in.

I see, thanks for the clear explanation!

Maybe this can be documented, since the logic is split in various places, it does not seem very clear to me at first glance.
(Unless some documentation already exists, in that case feel free to ignore).

Agree with you. (Seems don't have a related doc)

src/frontend/src/optimizer/mod.rs

liurenjie1024

Generally LGTM, just require some refinement.

src/frontend/src/handler/query.rs

liurenjie1024 · 2023-03-02T07:15:33Z

src/frontend/src/scheduler/local.rs

+                Ok(vec![self.front_env.worker_node_manager().next_random()?])
+            }
+        } else {
+            Ok(self.front_env.worker_node_manager().list_worker_nodes())


I think it should be workers with target table only.

liurenjie1024

LGTM

github-actions bot added type/feature user-facing-changes Contains changes that are visible to users labels Feb 27, 2023

ZENOTME changed the title ~~feat(frontend): support execute insert in local mode~~ (draft)feat(frontend): support execute insert in local mode Feb 28, 2023

github-actions bot added Invalid PR Title and removed type/feature labels Feb 28, 2023

ZENOTME force-pushed the zj/local_insert branch from 5435eb3 to a9b9424 Compare February 28, 2023 08:00

ZENOTME changed the title ~~(draft)feat(frontend): support execute insert in local mode~~ feat(frontend): support execute insert in local mode Feb 28, 2023

ZENOTME force-pushed the zj/local_insert branch from a9b9424 to e4420cf Compare February 28, 2023 08:24

ZENOTME removed the Invalid PR Title label Feb 28, 2023

ZENOTME force-pushed the zj/local_insert branch 2 times, most recently from b128755 to 2c9da12 Compare March 1, 2023 09:05

ZENOTME force-pushed the zj/local_insert branch from 2c9da12 to 76c0d4a Compare March 1, 2023 09:33

ZENOTME requested review from kwannoel, BugenZhao and liurenjie1024 and removed request for kwannoel March 1, 2023 09:34

ZENOTME closed this Mar 2, 2023

ZENOTME reopened this Mar 2, 2023

kwannoel reviewed Mar 2, 2023

View reviewed changes

src/frontend/src/handler/query.rs Show resolved Hide resolved

kwannoel reviewed Mar 2, 2023

View reviewed changes

src/frontend/src/optimizer/mod.rs Show resolved Hide resolved

liurenjie1024 reviewed Mar 2, 2023

View reviewed changes

ZENOTME added 2 commits March 2, 2023 22:19

rename modify.slt to dml.slt

c688a93

support insert in local mode

45b25a0

ZENOTME force-pushed the zj/local_insert branch from 76c0d4a to 45b25a0 Compare March 2, 2023 14:20

github-actions bot added the type/feature label Mar 3, 2023

liurenjie1024 approved these changes Mar 3, 2023

View reviewed changes

ZENOTME added this pull request to the merge queue Mar 3, 2023

Merged via the queue into main with commit 64810b2 Mar 3, 2023

ZENOTME deleted the zj/local_insert branch March 3, 2023 06:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): support execute insert in local mode #8208

feat(frontend): support execute insert in local mode #8208

ZENOTME commented Feb 27, 2023 •

edited

Loading

codecov bot commented Mar 1, 2023 •

edited

Loading

lmatz commented Mar 1, 2023

ZENOTME commented Mar 2, 2023

lmatz commented Mar 2, 2023

kwannoel Mar 2, 2023 •

edited

Loading

liurenjie1024 Mar 2, 2023 •

edited by ZENOTME

Loading

ZENOTME Mar 2, 2023 •

edited

Loading

kwannoel Mar 2, 2023

ZENOTME Mar 2, 2023 •

edited

Loading

kwannoel Mar 2, 2023

ZENOTME Mar 2, 2023

liurenjie1024 left a comment

liurenjie1024 Mar 2, 2023 •

edited by ZENOTME

Loading

liurenjie1024 left a comment

feat(frontend): support execute insert in local mode #8208

feat(frontend): support execute insert in local mode #8208

Conversation

ZENOTME commented Feb 27, 2023 • edited Loading

What's changed and what's your intention?

Checklist For Contributors

Checklist For Reviewers

Documentation

Types of user-facing changes

Release note

codecov bot commented Mar 1, 2023 • edited Loading

Codecov Report

lmatz commented Mar 1, 2023

ZENOTME commented Mar 2, 2023

lmatz commented Mar 2, 2023

kwannoel Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

liurenjie1024 Mar 2, 2023 • edited by ZENOTME Loading

Choose a reason for hiding this comment

ZENOTME Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

kwannoel Mar 2, 2023

Choose a reason for hiding this comment

ZENOTME Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

kwannoel Mar 2, 2023

Choose a reason for hiding this comment

ZENOTME Mar 2, 2023

Choose a reason for hiding this comment

liurenjie1024 left a comment

Choose a reason for hiding this comment

liurenjie1024 Mar 2, 2023 • edited by ZENOTME Loading

Choose a reason for hiding this comment

liurenjie1024 left a comment

Choose a reason for hiding this comment

ZENOTME commented Feb 27, 2023 •

edited

Loading

codecov bot commented Mar 1, 2023 •

edited

Loading

kwannoel Mar 2, 2023 •

edited

Loading

liurenjie1024 Mar 2, 2023 •

edited by ZENOTME

Loading

ZENOTME Mar 2, 2023 •

edited

Loading

ZENOTME Mar 2, 2023 •

edited

Loading

liurenjie1024 Mar 2, 2023 •

edited by ZENOTME

Loading