feat(config): deny unitialized data_directory and state_store

[Gun9niR]

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

What's changed and what's your intention?

To support mandatory initialization of a system parameter, developers can specify None as the default value in the params declaration macro (for_all_undeprecated_params). Then we can utilize system_params_to_kv invoked when the cluster is started for the first time to check for uninitialized params.

ALTER SYSTEM SET data_directory/state_store = DEFAULT; will also result in an error (though the semantic is wrong as discussed in #8851):

SystemParams error: data_directory does not have a default value

Closes #9153

Checklist For Contributors

• I have written necessary rustdoc comments
• 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

Types of user-facing changes

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

• Installation and deployment
• RisingWave cluster configuration changes

Release note

--data_directory and --state_store must be specified on CLI of the meta node, or the cluster will fail to start.

[Gun9niR]

@huangjw806 We might want to update the docs about system params.

/// Macro input is { field identifier, type, default value }
///
/// Note:
/// - Having `None` as default value means the parameter must be initialized.
macro_rules! for_all_undeprecated_params {
    ($macro:ident
        $(, { $field:ident, $type:ty, $default:expr })*) => {
        $macro! {
            { barrier_interval_ms, u32, Some(1000_u32) },
            { checkpoint_frequency, u64, Some(10_u64) },
            { sstable_size_mb, u32, Some(256_u32) },
            { block_size_kb, u32, Some(64_u32) },
            { bloom_false_positive, f64, Some(0.001_f64) },
            { state_store, String, None },
            { data_directory, String, None },
            { backup_storage_url, String, Some("memory".to_string()) },
            { backup_storage_directory, String, Some("backup".to_string()) },
            { telemetry_enabled, bool, Some(true) },
            $({ $field, $type, $default },)*
        }
    };
}

The third column's value being None means that the corresponding parameter must be specified from CLI.

[codecov]

Codecov Report

Merging #9170 (11debdc) into main (49630b2) will increase coverage by 0.00%.
The diff coverage is 48.88%.

@@           Coverage Diff           @@
##             main    #9170   +/-   ##
=======================================
  Coverage   70.84%   70.84%           
=======================================
  Files        1200     1200           
  Lines      199998   200010   +12     
=======================================
+ Hits       141689   141703   +14     
+ Misses      58309    58307    -2     

Flag	Coverage Δ
rust	70.84% <48.88%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
src/cmd_all/src/playground.rs	0.00% <0.00%> (ø)
src/common/src/system_param/local_manager.rs	90.00% <0.00%> (ø)
src/meta/src/lib.rs	0.90% <ø> (ø)
src/risedevtool/src/risectl_env.rs	0.00% <0.00%> (ø)
src/risedevtool/src/task/meta_node_service.rs	0.00% <0.00%> (ø)
src/risedevtool/src/task/utils.rs	0.00% <0.00%> (ø)
src/common/src/config.rs	78.64% <50.00%> (ø)
src/common/src/system_param/mod.rs	88.14% <90.00%> (-0.41%)	⬇️
...c/meta/src/backup_restore/meta_snapshot_builder.rs	87.50% <100.00%> (ø)
src/meta/src/manager/env.rs	69.49% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

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

[huangjw806]

@arkbriar Please confirm whether the operator needs to be changed synchronously, thank you!

[BugenZhao]

LGTM

[arkbriar]

LGTM

[arkbriar]

Please confirm whether the operator needs to be changed synchronously, thank you!

Sure. AFAIK, the operator should work without any change.