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

Code before:
}

/// Decide the memory limit for each storage cache. If not specified in `StorageConfig`, memory
/// limits are calculated based on the proportions to total `non_reserved_memory_bytes`.
pub fn storage_memory_config(
    non_reserved_memory_bytes: usize,
    embedded_compactor_enabled: bool,
    storage_config: &StorageConfig,
    is_serving: bool,
) -> StorageMemoryConfig {
    let (default_storage_memory_proportion, default_compactor_memory_proportion) = if embedded_compactor_enabled {
        (STORAGE_MEMORY_DEFAULT_PROPORTION, COMPACTOR_MEMORY_DEFAULT_PROPORTION)
    } else {
        (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::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 _),

Review comment (performance):
The comment states "all cache capacities" but the validation now includes compactor_memory_limit_mb in addition to the three cache capacities. Consider updating the comment to clarify that the validation includes both cache capacities and compactor memory, for example: "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."
```suggestion
    // 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.
```

Code after:
}

/// Decide the memory limit for each storage cache. If not specified in `StorageConfig`, memory
/// limits are calculated based on the proportions to total `non_reserved_memory_bytes`.
pub fn storage_memory_config(
    non_reserved_memory_bytes: usize,
    embedded_compactor_enabled: bool,
    storage_config: &StorageConfig,
    is_serving: bool,
) -> StorageMemoryConfig {
    let (default_storage_memory_proportion, default_compactor_memory_proportion) =
        if embedded_compactor_enabled {
            (
                STORAGE_MEMORY_DEFAULT_PROPORTION,
                COMPACTOR_MEMORY_DEFAULT_PROPORTION,
            )
        } else {
            (
                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,
    ) {
        (