feat: use `SHOW PARAMETERS` to list all system parameters #7882

Gun9niR · 2023-02-13T10:51:23Z

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

What's changed and what's your intention?

As title. Also refactored the code with macro to reduce the occurrence of hard coded keys such as CHECKPOINT_FREQUENCY_KEY.

dev=> show parameters;
           Name           |    Value    
--------------------------+-------------
 barrier_interval_ms      | 1000
 checkpoint_interval      | 10
 sstable_size_mb          | 256
 block_size_kb            | 64
 bloom_false_positive     | 0.001
 state_store              | 
 data_directory           | hummock_001
 backup_storage_url       | memory
 backup_storage_directory | backup

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
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)

Documentation

Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

SQL commands, functions, and operators

Release note

Use SHOW PARAMETERS; to show all system parameters.

src/common/src/system_param.rs

fuyufjh

Generally LGTM

src/common/src/system_param.rs

fuyufjh · 2023-02-14T04:38:54Z

src/meta/src/manager/system_param/model.rs

    async fn insert<S>(&self, store: &S) -> MetadataModelResult<()>
    where
        S: MetaStore,
    {
        let mut txn = Transaction::default();
-        self.barrier_interval_ms.inspect(|v| {


BugenZhao

LGTM

src/common/src/system_param.rs

codecov · 2023-02-14T09:52:38Z

Codecov Report

Merging #7882 (76d08bc) into main (680bc23) will increase coverage by 0.02%.
The diff coverage is 39.76%.

@@            Coverage Diff             @@
##             main    #7882      +/-   ##
==========================================
+ Coverage   71.78%   71.80%   +0.02%     
==========================================
  Files        1114     1116       +2     
  Lines      178474   178506      +32     
==========================================
+ Hits       128115   128185      +70     
+ Misses      50359    50321      -38

Flag	Coverage Δ
rust	`71.80% <39.76%> (+0.02%)`	⬆️

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

Impacted Files	Coverage Δ
src/common/src/error.rs	`71.64% <ø> (+1.09%)`	⬆️
src/common/src/lib.rs	`100.00% <ø> (ø)`
src/frontend/src/handler/mod.rs	`61.11% <0.00%> (ø)`
src/frontend/src/handler/variable.rs	`17.50% <0.00%> (-8.11%)`	⬇️
src/frontend/src/meta_client.rs	`0.00% <0.00%> (ø)`
src/frontend/src/test_utils.rs	`83.39% <0.00%> (-0.45%)`	⬇️
src/meta/src/manager/system_param/mod.rs	`0.00% <ø> (ø)`
src/meta/src/manager/system_param/model.rs	`0.00% <0.00%> (ø)`
src/meta/src/rpc/server.rs	`0.00% <0.00%> (ø)`
src/meta/src/rpc/service/cluster_service.rs	`0.00% <0.00%> (ø)`
... and 13 more

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

hengm3467 · 2023-02-21T02:23:05Z

@Gun9niR @fuyufjh I'm wondering if we have the definitions of these parameters (and how to use) documented somewhere? This one should be documented together with ALTER SYSTEM. I need a bit more info than just the command and the parameter names :)
I see we have an internal document about system parameters, but there are no value ranges. https://www.notion.so/risingwave-labs/System-Parameters-b9b9c00308bc4020a4208fa1764b24db?pvs=4

Thanks a lot!

fuyufjh · 2023-02-21T03:58:26Z

@Gun9niR @fuyufjh I'm wondering if we have the definitions of these parameters (and how to use) documented somewhere? This one should be documented together with ALTER SYSTEM. I need a bit more info than just the command and the parameter names :) I see we have an internal document about system parameters, but there are no value ranges. https://www.notion.so/risingwave-labs/System-Parameters-b9b9c00308bc4020a4208fa1764b24db?pvs=4

Thanks a lot!

@hengm3467 Please check system_param.rs for the source of truth. (You might need to switch to the correct branch other than main)

It's worth mentioning that

risingwave/src/common/src/system_param.rs

Lines 138 to 140 in ffce86a

    
           // Define check rules when a field is changed. By default all fields are immutable. 
        
           // If you want custom rules, please override the default implementation in 
        
           // `OverrideValidateOnSet` below.

So a param is either immutable or has a validation range defined there.

Gun9niR · 2023-02-21T05:42:20Z

@hengm3467 You can check for_all_undeprecated_params macro definition for a full list of params in the latest release (deprecated params will be hidden from SQL interface).

For value range/mutability, you can check OverrideValidateOnSet implementation. The absence of the function with a particular field name indicates that it is immutable.

xxchan · 2023-06-28T18:15:39Z

src/meta/src/manager/system_param/model.rs

-const SYSTEM_PARAM_CF_NAME: &str = "cf/system_params";
-
-const BARRIER_INTERVAL_MS_KEY: &str = "barrier_interval_ms";
-const CHECKPOINT_FREQUENCY_KEY: &str = "checkpoint_interval";


This is changed. Is this expected? 🤣

https://github.com/risingwavelabs/risingwave-docs/pull/1002

These hard-coded constants were only used for a few commits before I refactored the code to use macro to generate these keys (const CHECKPOINT_FREQUENCY_KEY: &str = "checkpoint_interval"; seems to be a mistake here). The key in etcd has been checkpoint_frequency since this commit.

support reading params

74f6895

github-actions bot added the type/feature label Feb 13, 2023

Gun9niR marked this pull request as draft February 13, 2023 11:13

Gun9niR changed the title ~~feat: SHOW PARAMETERS~~ feat: use SHOW PARAMETERS to see all system parameters Feb 13, 2023

use macro to refactor from kv

ecc8c25

Gun9niR added the user-facing-changes Contains changes that are visible to users label Feb 13, 2023

Gun9niR added 2 commits February 13, 2023 12:26

add ut

45ccd49

misc

eb4d3d5

Gun9niR requested review from fuyufjh and BugenZhao February 13, 2023 12:30

Gun9niR marked this pull request as ready for review February 13, 2023 12:30

Gun9niR changed the title ~~feat: use SHOW PARAMETERS to see all system parameters~~ feat: use SHOW PARAMETERS to list all system parameters Feb 13, 2023

BugenZhao reviewed Feb 13, 2023

View reviewed changes

src/common/src/system_param.rs Outdated Show resolved Hide resolved

Gun9niR added 2 commits February 13, 2023 16:29

further simplify key declaration with macro

07578ba

fix typo

b24629a

fuyufjh reviewed Feb 14, 2023

View reviewed changes

replace for_all_params with for_all_deprecated_params

71b2f88

fuyufjh approved these changes Feb 14, 2023

View reviewed changes

BugenZhao approved these changes Feb 14, 2023

View reviewed changes

src/common/src/system_param.rs Outdated Show resolved Hide resolved

use constant to denote mutability

d11dff8

Gun9niR added the mergify/can-merge label Feb 14, 2023

fix proto compatibility

2ac54df

Gun9niR force-pushed the zhidong/show-params branch from 83c4d48 to 2ac54df Compare February 14, 2023 07:29

Gun9niR added 3 commits February 14, 2023 15:30

Merge branch 'main' into zhidong/show-params

66f9d9f

Merge branch 'main' into zhidong/show-params

9107ea6

Merge branch 'main' into zhidong/show-params

a2a41ae

Merge branch 'main' into zhidong/show-params

76d08bc

mergify bot merged commit 7724305 into main Feb 14, 2023

mergify bot deleted the zhidong/show-params branch February 14, 2023 10:25

Gun9niR mentioned this pull request Feb 15, 2023

Tracking: System Parameters on Meta #7381

Closed

9 tasks

xxchan reviewed Jun 28, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use `SHOW PARAMETERS` to list all system parameters #7882

feat: use `SHOW PARAMETERS` to list all system parameters #7882

Gun9niR commented Feb 13, 2023 •

edited

Loading

fuyufjh left a comment

fuyufjh Feb 14, 2023

BugenZhao left a comment

codecov bot commented Feb 14, 2023 •

edited

Loading

hengm3467 commented Feb 21, 2023

fuyufjh commented Feb 21, 2023 •

edited

Loading

Gun9niR commented Feb 21, 2023 •

edited

Loading

xxchan Jun 28, 2023

Gun9niR Jun 29, 2023 •

edited

Loading

feat: use SHOW PARAMETERS to list all system parameters #7882

feat: use SHOW PARAMETERS to list all system parameters #7882

Conversation

Gun9niR commented Feb 13, 2023 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Types of user-facing changes

Release note

fuyufjh left a comment

Choose a reason for hiding this comment

fuyufjh Feb 14, 2023

Choose a reason for hiding this comment

BugenZhao left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 14, 2023 • edited Loading

Codecov Report

hengm3467 commented Feb 21, 2023

fuyufjh commented Feb 21, 2023 • edited Loading

Gun9niR commented Feb 21, 2023 • edited Loading

xxchan Jun 28, 2023

Choose a reason for hiding this comment

Gun9niR Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

feat: use `SHOW PARAMETERS` to list all system parameters #7882

feat: use `SHOW PARAMETERS` to list all system parameters #7882

Gun9niR commented Feb 13, 2023 •

edited

Loading

codecov bot commented Feb 14, 2023 •

edited

Loading

fuyufjh commented Feb 21, 2023 •

edited

Loading

Gun9niR commented Feb 21, 2023 •

edited

Loading

Gun9niR Jun 29, 2023 •

edited

Loading