sentinel-group / sentinel-rust

Sentinel Rust version
Apache License 2.0
132 stars 22 forks source link

Incorrect Usage of Global Config in Metric Log Initialization #139

Open flearc opened 8 months ago

flearc commented 8 months ago

The global config in the sentinel-core module is defined as a thread-local variable:

thread_local! {
    static GLOBAL_CONFIG : RefCell<ConfigEntity> = RefCell::new(ConfigEntity::new());
}

However, during the initialization of metric logs in the init_core_components function, a new thread is spawned to perform the task, which will use the default value of the global config:

// `init_core_components` init core components with global config
#[inline]
fn init_core_components() -> Result<()> {
    if config::metric_log_flush_interval_sec() > 0 {
        #[cfg(feature = "metric_log")]
        metric::init_task();
    }
    ...
}
pub fn init_task() {
    INIT_ONCE.call_once(|| {
        std::thread::spawn(|| loop {
            do_aggregate();
            sleep_for_ms((config::metric_log_flush_interval_sec() * 1000).into());
        });
    });
}

Functions like remove_deprecated_files and roll_file_size_exceeded in the metric log writer module will not behave as expected when users define a custom config.

Forsworns commented 7 months ago

I think a lazy_static variable may be more suitable for the configuration items.