* [pbs-devel] [PATCH proxmox-backup 1/3] backup/datastore: count still bad chunks for the status
2020-10-23 14:32 [pbs-devel] [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api Dominik Csapak
@ 2020-10-23 14:32 ` Dominik Csapak
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 2/3] backup/datastore: save garbage collection status to disk Dominik Csapak
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2020-10-23 14:32 UTC (permalink / raw)
To: pbs-devel
we want to show the user that there are still bad chunks after a garbage
collection
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/types/mod.rs | 3 +++
src/backup/chunk_store.rs | 2 ++
2 files changed, 5 insertions(+)
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index f97db557..1b9a305f 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -587,6 +587,8 @@ pub struct GarbageCollectionStatus {
pub pending_chunks: usize,
/// Number of chunks marked as .bad by verify that have been removed by GC.
pub removed_bad: usize,
+ /// Number of chunks still marked as .bad after garbage collection.
+ pub still_bad: usize,
}
impl Default for GarbageCollectionStatus {
@@ -602,6 +604,7 @@ impl Default for GarbageCollectionStatus {
pending_bytes: 0,
pending_chunks: 0,
removed_bad: 0,
+ still_bad: 0,
}
}
}
diff --git a/src/backup/chunk_store.rs b/src/backup/chunk_store.rs
index 96c46efb..b7556f8a 100644
--- a/src/backup/chunk_store.rs
+++ b/src/backup/chunk_store.rs
@@ -354,9 +354,11 @@ impl ChunkStore {
},
Err(nix::Error::Sys(nix::errno::Errno::ENOENT)) => {
// chunk hasn't been rewritten yet, keep .bad file
+ status.still_bad += 1;
},
Err(err) => {
// some other error, warn user and keep .bad file around too
+ status.still_bad += 1;
crate::task_warn!(
worker,
"error during stat on '{:?}' - {}",
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/3] backup/datastore: save garbage collection status to disk
2020-10-23 14:32 [pbs-devel] [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api Dominik Csapak
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 1/3] backup/datastore: count still bad chunks for the status Dominik Csapak
@ 2020-10-23 14:32 ` Dominik Csapak
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 3/3] admin/datastore: add more info to status call Dominik Csapak
2020-10-27 16:44 ` [pbs-devel] applied-series: [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api Thomas Lamprecht
3 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2020-10-23 14:32 UTC (permalink / raw)
To: pbs-devel
and load it again when opening it
this way we can persist the status of the last garbage collect across
daemon reloads and reboots
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/backup/datastore.rs | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
index 2389cc6c..485a10ff 100644
--- a/src/backup/datastore.rs
+++ b/src/backup/datastore.rs
@@ -9,7 +9,7 @@ use std::fs::File;
use anyhow::{bail, format_err, Error};
use lazy_static::lazy_static;
-use proxmox::tools::fs::{replace_file, CreateOptions, open_file_locked};
+use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions, open_file_locked};
use super::backup_info::{BackupGroup, BackupDir};
use super::chunk_store::ChunkStore;
@@ -71,7 +71,20 @@ impl DataStore {
fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<Self, Error> {
let chunk_store = ChunkStore::open(store_name, path)?;
- let gc_status = GarbageCollectionStatus::default();
+ let mut gc_status_path = chunk_store.base_path();
+ gc_status_path.push(".gc-status");
+
+ let gc_status = if let Some(state) = file_read_optional_string(gc_status_path)? {
+ match serde_json::from_str(&state) {
+ Ok(state) => state,
+ Err(err) => {
+ eprintln!("error reading gc-status: {}", err);
+ GarbageCollectionStatus::default()
+ }
+ }
+ } else {
+ GarbageCollectionStatus::default()
+ };
Ok(Self {
chunk_store: Arc::new(chunk_store),
@@ -572,6 +585,23 @@ impl DataStore {
crate::task_log!(worker, "Average chunk size: {}", HumanByte::from(avg_chunk));
}
+ if let Ok(serialized) = serde_json::to_string(&gc_status) {
+ let mut path = self.base_path();
+ path.push(".gc-status");
+
+ let backup_user = crate::backup::backup_user()?;
+ let mode = nix::sys::stat::Mode::from_bits_truncate(0o0644);
+ // set the correct owner/group/permissions while saving file
+ // owner(rw) = backup, group(r)= backup
+ let options = CreateOptions::new()
+ .perm(mode)
+ .owner(backup_user.uid)
+ .group(backup_user.gid);
+
+ // ignore errors
+ let _ = replace_file(path, serialized.as_bytes(), options);
+ }
+
*self.last_gc_status.lock().unwrap() = gc_status;
} else {
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/3] admin/datastore: add more info to status call
2020-10-23 14:32 [pbs-devel] [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api Dominik Csapak
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 1/3] backup/datastore: count still bad chunks for the status Dominik Csapak
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 2/3] backup/datastore: save garbage collection status to disk Dominik Csapak
@ 2020-10-23 14:32 ` Dominik Csapak
2020-11-03 5:52 ` Dietmar Maurer
2020-10-27 16:44 ` [pbs-devel] applied-series: [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api Thomas Lamprecht
3 siblings, 1 reply; 7+ messages in thread
From: Dominik Csapak @ 2020-10-23 14:32 UTC (permalink / raw)
To: pbs-devel
add also the snapshot counts as well as the status of the last garbage
collection
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/admin/datastore.rs | 61 +++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 3 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index feeff8af..e1a0aacc 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -423,6 +423,37 @@ pub fn list_snapshots (
Ok(snapshots)
}
+// returns a map from type to (group_count, snapshot_count)
+fn get_snaphots_count(store: &DataStore) -> Result<HashMap<String, (usize, usize)>, Error> {
+ let base_path = store.base_path();
+ let backup_list = BackupInfo::list_backups(&base_path)?;
+ let mut groups = HashSet::new();
+ let mut result: HashMap<String, (usize, usize)> = HashMap::new();
+ for info in backup_list {
+ let group = info.backup_dir.group();
+
+ let id = group.backup_id();
+ let backup_type = group.backup_type();
+
+ let mut new_id = false;
+
+ if groups.insert(format!("{}-{}", &backup_type, &id)) {
+ new_id = true;
+ }
+
+ if let Some(mut counts) = result.get_mut(backup_type) {
+ counts.1 += 1;
+ if new_id {
+ counts.0 +=1;
+ }
+ } else {
+ result.insert(backup_type.to_string(), (1, 1));
+ }
+ }
+
+ Ok(result)
+}
+
#[api(
input: {
properties: {
@@ -432,7 +463,21 @@ pub fn list_snapshots (
},
},
returns: {
- type: StorageStatus,
+ description: "The overall Datastore status and information.",
+ type: Object,
+ properties: {
+ storage: {
+ type: StorageStatus,
+ },
+ counts: {
+ description: "Group and Snapshot counts per Type",
+ type: Object,
+ properties: { },
+ },
+ "gc-status": {
+ type: GarbageCollectionStatus,
+ },
+ },
},
access: {
permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP, true),
@@ -443,9 +488,19 @@ pub fn status(
store: String,
_info: &ApiMethod,
_rpcenv: &mut dyn RpcEnvironment,
-) -> Result<StorageStatus, Error> {
+) -> Result<Value, Error> {
let datastore = DataStore::lookup_datastore(&store)?;
- crate::tools::disks::disk_usage(&datastore.base_path())
+ let storage_status = crate::tools::disks::disk_usage(&datastore.base_path())?;
+ let counts = get_snaphots_count(&datastore)?;
+ let gc_status = datastore.last_gc_status();
+
+ let res = json!({
+ "storage": storage_status,
+ "counts": counts,
+ "gc-status": gc_status,
+ });
+
+ Ok(res)
}
#[api(
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/3] admin/datastore: add more info to status call
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 3/3] admin/datastore: add more info to status call Dominik Csapak
@ 2020-11-03 5:52 ` Dietmar Maurer
2020-11-03 6:53 ` Dominik Csapak
0 siblings, 1 reply; 7+ messages in thread
From: Dietmar Maurer @ 2020-11-03 5:52 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
This change makes the API call very slow. because it scans over the whole directory tree.
Is this really necessary?
> On 10/23/2020 4:32 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
>
>
> add also the snapshot counts as well as the status of the last garbage
> collection
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> src/api2/admin/datastore.rs | 61 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index feeff8af..e1a0aacc 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -423,6 +423,37 @@ pub fn list_snapshots (
> Ok(snapshots)
> }
>
> +// returns a map from type to (group_count, snapshot_count)
> +fn get_snaphots_count(store: &DataStore) -> Result<HashMap<String, (usize, usize)>, Error> {
> + let base_path = store.base_path();
> + let backup_list = BackupInfo::list_backups(&base_path)?;
> + let mut groups = HashSet::new();
> + let mut result: HashMap<String, (usize, usize)> = HashMap::new();
> + for info in backup_list {
> + let group = info.backup_dir.group();
> +
> + let id = group.backup_id();
> + let backup_type = group.backup_type();
> +
> + let mut new_id = false;
> +
> + if groups.insert(format!("{}-{}", &backup_type, &id)) {
> + new_id = true;
> + }
> +
> + if let Some(mut counts) = result.get_mut(backup_type) {
> + counts.1 += 1;
> + if new_id {
> + counts.0 +=1;
> + }
> + } else {
> + result.insert(backup_type.to_string(), (1, 1));
> + }
> + }
> +
> + Ok(result)
> +}
> +
> #[api(
> input: {
> properties: {
> @@ -432,7 +463,21 @@ pub fn list_snapshots (
> },
> },
> returns: {
> - type: StorageStatus,
> + description: "The overall Datastore status and information.",
> + type: Object,
> + properties: {
> + storage: {
> + type: StorageStatus,
> + },
> + counts: {
> + description: "Group and Snapshot counts per Type",
> + type: Object,
> + properties: { },
> + },
> + "gc-status": {
> + type: GarbageCollectionStatus,
> + },
> + },
> },
> access: {
> permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP, true),
> @@ -443,9 +488,19 @@ pub fn status(
> store: String,
> _info: &ApiMethod,
> _rpcenv: &mut dyn RpcEnvironment,
> -) -> Result<StorageStatus, Error> {
> +) -> Result<Value, Error> {
> let datastore = DataStore::lookup_datastore(&store)?;
> - crate::tools::disks::disk_usage(&datastore.base_path())
> + let storage_status = crate::tools::disks::disk_usage(&datastore.base_path())?;
> + let counts = get_snaphots_count(&datastore)?;
> + let gc_status = datastore.last_gc_status();
> +
> + let res = json!({
> + "storage": storage_status,
> + "counts": counts,
> + "gc-status": gc_status,
> + });
> +
> + Ok(res)
> }
>
> #[api(
> --
> 2.20.1
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/3] admin/datastore: add more info to status call
2020-11-03 5:52 ` Dietmar Maurer
@ 2020-11-03 6:53 ` Dominik Csapak
0 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2020-11-03 6:53 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On 11/3/20 6:52 AM, Dietmar Maurer wrote:
> This change makes the API call very slow. because it scans over the whole directory tree.
> Is this really necessary?
>
well, not necessary, but it provides some good information to show to
the user, maybe we can cache that information somehow?
(previously we showed the list of snapshots by default, which is even
more work & slower)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pbs-devel] applied-series: [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api
2020-10-23 14:32 [pbs-devel] [PATCH proxmox-backup 0/3] improve and extend admin/datastore/status api Dominik Csapak
` (2 preceding siblings ...)
2020-10-23 14:32 ` [pbs-devel] [PATCH proxmox-backup 3/3] admin/datastore: add more info to status call Dominik Csapak
@ 2020-10-27 16:44 ` Thomas Lamprecht
3 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2020-10-27 16:44 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
On 23.10.20 16:32, Dominik Csapak wrote:
> i'll need that for the pbs datastore summary, sending it now since the ui is
> not completely done yet
>
> Dominik Csapak (3):
> backup/datastore: count still bad chunks for the status
> backup/datastore: save garbage collection status to disk
> admin/datastore: add more info to status call
>
> src/api2/admin/datastore.rs | 61 +++++++++++++++++++++++++++++++++++--
> src/api2/types/mod.rs | 3 ++
> src/backup/chunk_store.rs | 2 ++
> src/backup/datastore.rs | 34 +++++++++++++++++++--
> 4 files changed, 95 insertions(+), 5 deletions(-)
>
applied series, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread