Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[823] add detailed disk metrics #827

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Bob versions changelog
- Added mimalloc allocator for musl target (#688)
- Added jemalloc-profile for memory profiling (#797)
- Proper support for GetSource::ALL requests (#723)
- Added detailed information about total, used and free space on every disk (#823)

#### Changed
- BobClient clone overhead reduced (#774)
Expand Down
76 changes: 60 additions & 16 deletions bob/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ use bob_access::{Authenticator, CredentialsHolder};
use bob_backend::pearl::{Group as PearlGroup, Holder, NoopHooks};
use bob_common::{
configs::node::TLSConfig,
core_types::{DiskName, NodeDisk, VDisk as DataVDisk},
data::{BobData, BobKey, BobMeta, BOB_KEY_SIZE},
core_types::{VDisk as DataVDisk, NodeDisk},
operation_options::{BobPutOptions, BobGetOptions, BobDeleteOptions},
error::Error as BobError,
operation_options::{BobDeleteOptions, BobGetOptions, BobPutOptions},
};
use bytes::Bytes;
use futures::{future::BoxFuture, stream::FuturesUnordered, FutureExt, StreamExt};
Expand Down Expand Up @@ -145,12 +145,18 @@ pub(crate) struct NodeConfiguration {
}

#[derive(Debug, Serialize)]
pub(crate) struct SpaceInfo {
struct Space {
total_disk_space_bytes: u64,
free_disk_space_bytes: u64,
used_disk_space_bytes: u64,
occupied_disk_space_bytes: u64,
Copy link
Member

@ikopylov ikopylov Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we will decide to allow breaking changes, then this field should be renamed to occupied_by_bob_disk_space_bytes. Such a name will better reflect the meaning of the field

occupied_disk_space_by_disk: HashMap<String, u64>,
}

#[derive(Debug, Serialize)]
pub(crate) struct SpaceInfo {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update openapi in config-examples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to integrate utoipa for openapi auto-gen at some point

#[serde(flatten)]
total_space: Space,
disk_space_by_disk: HashMap<DiskName, Space>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this is a breaking change.

@idruzhitskiy can we support both old and this new formats in OldPartitionsRemover?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another problem here: you try to combine logical and physical structure in this map. Probably, it is better to split them. occupied_disk_space_by_disk is created to represent space info within logical structure. I suggest to keep it without changes (occupied_disk_space_by_disk: HashMap<DiskName, u64>) and add additional disk_space_by_disk for physical structure (its key will be a mount point of the disk, not its name)

pub(crate) struct SpaceInfo {
    #[serde(flatten)]
    total_space: Space,
    occupied_disk_space_by_disk: HashMap<DiskName, u64>,  // key is a Bob disk name
    disk_space_by_disk: HashMap<String, Space>  // Key is mount point
}

}

async fn tls_server(tls_config: &TLSConfig, addr: SocketAddr) -> AxumServer<RustlsAcceptor> {
Expand Down Expand Up @@ -313,31 +319,69 @@ async fn status<A: Authenticator>(bob: Extension<BobServer<A>>) -> Json<Node> {
async fn get_space_info<A: Authenticator>(
bob: Extension<BobServer<A>>,
) -> Result<Json<SpaceInfo>, StatusExt> {
let mut disk_metrics = bob.grinder().hw_counter().update_space_metrics();
let DiskSpaceMetrics {
total_space,
used_space,
free_space,
} = bob.grinder().hw_counter().update_space_metrics();
} = disk_metrics.values().sum();
ikopylov marked this conversation as resolved.
Show resolved Hide resolved

let backend = bob.grinder().backend().inner();
let (dcs, adc) = backend
.disk_controllers()
.ok_or_else(not_acceptable_backend)?;
let mut map = HashMap::new();
for dc in dcs.iter() {
map.insert(dc.disk().name().to_string(), dc.disk_used().await);
let mut disk_space_by_disk = HashMap::new();
let mut total_occupied = 0;
for dc in dcs {
let occupied_space = dc.disk_used().await;
total_occupied += occupied_space;
let DiskSpaceMetrics {
total_space,
used_space,
free_space,
ikopylov marked this conversation as resolved.
Show resolved Hide resolved
} = disk_metrics
.remove_entry(dc.disk().name())
ikopylov marked this conversation as resolved.
Show resolved Hide resolved
.map(|(_, space)| space)
.unwrap_or_default();
disk_space_by_disk.insert(
dc.disk().name().clone(),
Space {
total_disk_space_bytes: total_space,
free_disk_space_bytes: free_space,
used_disk_space_bytes: used_space,
occupied_disk_space_bytes: occupied_space,
},
);
}
let adc_space = adc.disk_used().await;
archeoss marked this conversation as resolved.
Show resolved Hide resolved
map.entry(adc.disk().name().to_string())
.and_modify(|s| *s = *s + adc_space)
.or_insert(adc_space);
disk_space_by_disk
.entry(adc.disk().name().clone())
.and_modify(|s| s.occupied_disk_space_bytes += adc_space)
.or_insert({
let DiskSpaceMetrics {
total_space,
used_space,
free_space,
} = disk_metrics
.remove_entry(adc.disk().name())
.map(|(_, space)| space)
.unwrap_or_default();
ikopylov marked this conversation as resolved.
Show resolved Hide resolved
Space {
total_disk_space_bytes: total_space,
free_disk_space_bytes: free_space,
used_disk_space_bytes: used_space,
occupied_disk_space_bytes: adc_space,
}
});

Ok(Json(SpaceInfo {
total_disk_space_bytes: total_space,
used_disk_space_bytes: used_space,
free_disk_space_bytes: free_space,
occupied_disk_space_bytes: map.values().sum(),
occupied_disk_space_by_disk: map,
total_space: Space {
total_disk_space_bytes: total_space,
used_disk_space_bytes: used_space,
free_disk_space_bytes: free_space,
occupied_disk_space_bytes: total_occupied,
},
disk_space_by_disk,
}))
}

Expand Down
Loading