Repository: risingwavelabs/risingwave
File: src/compute/src/memory/config.rs (Rust)

Code before:
    let max_storage_memory_bytes = (non_reserved_memory_bytes as f64 * STORAGE_MEMORY_MAX_PROPORTION).ceil();

    let compactor_memory_limit_mb = storage_config.compactor_memory_limit_mb.unwrap_or(
        ((non_reserved_memory_bytes as f64 * default_compactor_memory_proportion).ceil() as usize) >> 20,
    );
    tracing::warn!("default_storage_memory_bytes={} max_storage_memory_bytes={} compactor_memory_limit_mb={}", default_storage_memory_bytes, max_storage_memory_bytes, compactor_memory_limit_mb);

    // Only if the all cache capacities are specified and their sum doesn't exceed the max allowed storage_memory_bytes, will we use them.
    // Other invalid combination of the cache capacities config will be ignored.
    let (
        config_block_cache_capacity_mb,
        config_meta_cache_capacity_mb,
        config_shared_buffer_capacity_mb,
    ) = match (
        storage_config.cache.block_cache_capacity_mb,
        storage_config.cache.meta_cache_capacity_mb,
        storage_config.shared_buffer_capacity_mb,
    ) {
        (
            Some(block_cache_capacity_mb),
            Some(meta_cache_capacity_mb),
            Some(shared_buffer_capacity_mb),
        ) => {
            let config_storage_memory_bytes =
                (block_cache_capacity_mb + meta_cache_capacity_mb + shared_buffer_capacity_mb + compactor_memory_limit_mb)
                    << 20;
            if config_storage_memory_bytes as f64 > max_storage_memory_bytes {
                tracing::warn!(
                    "config block_cache_capacity_mb {} + meta_cache_capacity_mb {} + shared_buffer_capacity_mb {}  + compactor_memory_limit_mb {} = {} exceeds allowed storage_memory_bytes {}. These configs will be ignored.",
                    block_cache_capacity_mb,
                    meta_cache_capacity_mb,
                    shared_buffer_capacity_mb,
                    compactor_memory_limit_mb,
                    convert(config_storage_memory_bytes as _),
                    convert(max_storage_memory_bytes as _)
                );
                (None, None, None)
            } else {
                (
                    Some(block_cache_capacity_mb),
                    Some(meta_cache_capacity_mb),
                    Some(shared_buffer_capacity_mb),
                )
            }
        }
        c => {
            tracing::warn!(
                "config (block_cache_capacity_mb, meta_cache_capacity_mb, shared_buffer_capacity_mb): {:?} should be set altogether. These configs will be ignored.",
                c
            );
            (None, None, None)

Review comment (performance):
The validation now includes compactor_memory_limit_mb in the total (line 144), which is a behavior change from the original code. Previously, compactor memory was calculated separately and not included in this validation check. This change should be explicitly tested to ensure the validation works correctly when users specify all four memory parameters (block_cache, meta_cache, shared_buffer, and compactor_memory_limit).

Code after:
            (
                STORAGE_MEMORY_DEFAULT_PROPORTION + COMPACTOR_MEMORY_DEFAULT_PROPORTION,
                0.0,
            )
        };

    let default_storage_memory_bytes =
        (non_reserved_memory_bytes as f64 * default_storage_memory_proportion).ceil();
    let max_storage_memory_bytes =
        (non_reserved_memory_bytes as f64 * STORAGE_MEMORY_MAX_PROPORTION).ceil();

    let compactor_memory_limit_mb = storage_config.compactor_memory_limit_mb.unwrap_or(
        ((non_reserved_memory_bytes as f64 * default_compactor_memory_proportion).ceil() as usize)
            >> 20,
    );
    tracing::debug!(
        "default_storage_memory_bytes={} max_storage_memory_bytes={} compactor_memory_limit_mb={}",
        default_storage_memory_bytes,
        max_storage_memory_bytes,
        compactor_memory_limit_mb
    );

    // Only if all cache capacities and compactor memory are specified and their sum doesn't exceed the max allowed storage_memory_bytes, will we use them.
    // Other invalid combinations of the cache capacities and compactor memory config will be ignored.
    let (
        config_block_cache_capacity_mb,
        config_meta_cache_capacity_mb,
        config_shared_buffer_capacity_mb,
    ) = match (
        storage_config.cache.block_cache_capacity_mb,
        storage_config.cache.meta_cache_capacity_mb,
        storage_config.shared_buffer_capacity_mb,
    ) {
        (
            Some(block_cache_capacity_mb),
            Some(meta_cache_capacity_mb),
            Some(shared_buffer_capacity_mb),
        ) => {
            let config_storage_memory_bytes = (block_cache_capacity_mb
                + meta_cache_capacity_mb
                + shared_buffer_capacity_mb
                + compactor_memory_limit_mb)
                << 20;
            if config_storage_memory_bytes as f64 > max_storage_memory_bytes {
                tracing::warn!(
                    "config block_cache_capacity_mb {} + meta_cache_capacity_mb {} + shared_buffer_capacity_mb {} + compactor_memory_limit_mb {} = {} exceeds allowed storage_memory_bytes {}. These configs will be ignored.",
                    block_cache_capacity_mb,
                    meta_cache_capacity_mb,
                    shared_buffer_capacity_mb,
                    compactor_memory_limit_mb,
                    convert(config_storage_memory_bytes as _),