* [pbs-devel] [PATCH v4 proxmox-backup 1/6] pbs-api-types: add maintenance type
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
@ 2021-11-12 12:30 ` Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 2/6] pbs-datastore: add check for maintenance in lookup Hannes Laimer
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2021-11-12 12:30 UTC (permalink / raw)
To: pbs-devel
---
pbs-api-types/src/datastore.rs | 8 +++-
pbs-api-types/src/lib.rs | 3 ++
pbs-api-types/src/maintenance.rs | 82 ++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 pbs-api-types/src/maintenance.rs
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 77c1258f..a12e20b3 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -7,7 +7,7 @@ use proxmox_schema::{
use crate::{
PROXMOX_SAFE_ID_FORMAT, SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA, CryptMode, UPID,
- Fingerprint, Userid, Authid,
+ Fingerprint, Userid, Authid, MaintenanceType,
GC_SCHEDULE_SCHEMA, DATASTORE_NOTIFY_STRING_SCHEMA, PRUNE_SCHEDULE_SCHEMA,
};
@@ -219,6 +219,10 @@ pub struct PruneOptions {
optional: true,
type: bool,
},
+ "maintenance-type": {
+ optional: true,
+ type: MaintenanceType,
+ },
}
)]
#[derive(Serialize,Deserialize,Updater)]
@@ -256,6 +260,8 @@ pub struct DataStoreConfig {
/// Send notification only for job errors
#[serde(skip_serializing_if="Option::is_none")]
pub notify: Option<String>,
+ #[serde(skip_serializing_if="Option::is_none")]
+ pub maintenance_type: Option<MaintenanceType>,
}
#[api(
diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index a61de960..fbdb57f7 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -45,6 +45,9 @@ pub use jobs::*;
mod key_derivation;
pub use key_derivation::{Kdf, KeyInfo};
+mod maintenance;
+pub use maintenance::*;
+
mod network;
pub use network::*;
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
new file mode 100644
index 00000000..f816b279
--- /dev/null
+++ b/pbs-api-types/src/maintenance.rs
@@ -0,0 +1,82 @@
+use anyhow::{bail, Error};
+
+use proxmox_schema::{parse_simple_value, ApiStringFormat, Schema, StringSchema, UpdaterType};
+
+use crate::{PROXMOX_SAFE_ID_FORMAT, SINGLE_LINE_COMMENT_FORMAT};
+
+pub const MAINTENANCE_MODE_SCHEMA: Schema = StringSchema::new("Maintenance mode.")
+ .format(&PROXMOX_SAFE_ID_FORMAT)
+ .min_length(3)
+ .max_length(32)
+ .schema();
+
+pub const MAINTENANCE_MESSAGE_SCHEMA: Schema =
+ StringSchema::new("Message describing the reason for the maintenance.")
+ .format(&SINGLE_LINE_COMMENT_FORMAT)
+ .max_length(32)
+ .schema();
+
+#[derive(UpdaterType)]
+/// Maintenance type and message.
+pub enum MaintenanceType {
+ /// Only reading operations are allowed on the datastore.
+ ReadOnly(String),
+ /// Neither reading nor writing operations are allowed on the datastore.
+ Offline(String),
+}
+
+/// Operation requirments, used when checking for maintenance mode.
+pub enum Operation {
+ Read,
+ Write,
+}
+
+proxmox::forward_deserialize_to_from_str!(MaintenanceType);
+proxmox::forward_serialize_to_display!(MaintenanceType);
+
+impl proxmox_schema::ApiType for MaintenanceType {
+ const API_SCHEMA: Schema = StringSchema::new(
+ "Maintenance type (e.g. 'read-only-<message>', 'offline-<message>')",
+ )
+ .format(&ApiStringFormat::VerifyFn(|text| {
+ let location: MaintenanceType = text.parse()?;
+ match location {
+ MaintenanceType::ReadOnly(ref message) => {
+ parse_simple_value(message, &MAINTENANCE_MESSAGE_SCHEMA)?;
+ }
+ MaintenanceType::Offline(ref message) => {
+ parse_simple_value(message, &MAINTENANCE_MESSAGE_SCHEMA)?;
+ }
+ }
+ Ok(())
+ }))
+ .schema();
+}
+
+impl std::fmt::Display for MaintenanceType {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ MaintenanceType::ReadOnly(message) => {
+ write!(f, "read-only-{}", message)
+ }
+ MaintenanceType::Offline(message) => {
+ write!(f, "offline-{}", message)
+ }
+ }
+ }
+}
+
+impl std::str::FromStr for MaintenanceType {
+ type Err = Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ if let Some(message) = s.strip_prefix("read-only-") {
+ return Ok(MaintenanceType::ReadOnly(message.to_string()));
+ }
+ if let Some(message) = s.strip_prefix("offline-") {
+ return Ok(MaintenanceType::Offline(message.to_string()));
+ }
+
+ bail!("Maintenance type parse error");
+ }
+}
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH v4 proxmox-backup 2/6] pbs-datastore: add check for maintenance in lookup
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 1/6] pbs-api-types: add maintenance type Hannes Laimer
@ 2021-11-12 12:30 ` Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 3/6] pbs-datastore: add active operations tracking Hannes Laimer
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2021-11-12 12:30 UTC (permalink / raw)
To: pbs-devel
---
pbs-datastore/src/datastore.rs | 17 +++++++---
pbs-datastore/src/snapshot_reader.rs | 6 +++-
src/api2/admin/datastore.rs | 48 ++++++++++++++--------------
src/api2/backup/mod.rs | 4 +--
src/api2/pull.rs | 4 +--
src/api2/reader/mod.rs | 6 ++--
src/api2/status.rs | 4 +--
src/api2/tape/backup.rs | 6 ++--
src/api2/tape/restore.rs | 6 ++--
src/bin/proxmox-backup-proxy.rs | 6 ++--
src/server/prune_job.rs | 4 +--
src/server/verify_job.rs | 4 +--
12 files changed, 64 insertions(+), 51 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 5049cb3d..064fd273 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -11,7 +11,7 @@ use lazy_static::lazy_static;
use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
-use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus};
+use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, MaintenanceType, Operation};
use pbs_tools::format::HumanByte;
use pbs_tools::fs::{lock_dir_noblock, DirLockGuard};
use pbs_tools::process_locker::ProcessLockSharedGuard;
@@ -60,13 +60,22 @@ pub struct DataStore {
}
impl DataStore {
-
- pub fn lookup_datastore(name: &str) -> Result<Arc<DataStore>, Error> {
-
+ pub fn lookup_datastore(
+ name: &str,
+ operation: Option<Operation>,
+ ) -> Result<Arc<DataStore>, Error> {
let (config, _digest) = pbs_config::datastore::config()?;
let config: DataStoreConfig = config.lookup("datastore", name)?;
let path = PathBuf::from(&config.path);
+ match (&config.maintenance_type, operation.clone()) {
+ (Some(MaintenanceType::ReadOnly(message)), Some(Operation::Write))
+ | (Some(MaintenanceType::Offline(message)), Some(_)) => {
+ bail!("Datastore '{}' is in maintenance mode: {}", name, message);
+ },
+ _ => {}
+ }
+
let mut map = DATASTORE_MAP.lock().unwrap();
if let Some(datastore) = map.get(name) {
diff --git a/pbs-datastore/src/snapshot_reader.rs b/pbs-datastore/src/snapshot_reader.rs
index c386256d..146bba71 100644
--- a/pbs-datastore/src/snapshot_reader.rs
+++ b/pbs-datastore/src/snapshot_reader.rs
@@ -12,6 +12,7 @@ use crate::fixed_index::FixedIndexReader;
use crate::dynamic_index::DynamicIndexReader;
use crate::manifest::{archive_type, ArchiveType, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME};
use crate::DataStore;
+use pbs_api_types::Operation;
use pbs_tools::fs::lock_dir_noblock_shared;
/// Helper to access the contents of a datastore backup snapshot
@@ -119,7 +120,10 @@ impl <'a> Iterator for SnapshotChunkIterator<'a> {
};
let datastore =
- DataStore::lookup_datastore(self.snapshot_reader.datastore_name())?;
+ DataStore::lookup_datastore(
+ self.snapshot_reader.datastore_name(),
+ Some(Operation::Read)
+ )?;
let order = datastore.get_chunks_in_order(&index, |_| false, |_| Ok(()))?;
self.current_index = Some((Arc::new(index), 0, order));
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index dcc89147..02a42369 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -27,7 +27,7 @@ use pxar::EntryKind;
use pbs_api_types::{ Authid, BackupContent, Counts, CryptMode,
DataStoreListItem, GarbageCollectionStatus, GroupListItem,
- SnapshotListItem, SnapshotVerifyState, PruneOptions,
+ Operation, SnapshotListItem, SnapshotVerifyState, PruneOptions,
DataStoreStatus, RRDMode, RRDTimeFrame,
BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_TIME_SCHEMA,
BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
@@ -170,7 +170,7 @@ pub fn list_groups(
let user_info = CachedUserInfo::new()?;
let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
let backup_groups = BackupInfo::list_backup_groups(&datastore.base_path())?;
@@ -268,7 +268,7 @@ pub fn delete_group(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let group = BackupGroup::new(backup_type, backup_id);
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
@@ -315,7 +315,7 @@ pub fn list_snapshot_files(
) -> Result<Vec<BackupContent>, Error> {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -365,7 +365,7 @@ pub fn delete_snapshot(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_MODIFY)?;
@@ -414,7 +414,7 @@ pub fn list_snapshots (
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let base_path = datastore.base_path();
@@ -611,7 +611,7 @@ pub fn status(
_info: &ApiMethod,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<DataStoreStatus, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
let (counts, gc_status) = if verbose {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -689,7 +689,7 @@ pub fn verify(
outdated_after: Option<i64>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let ignore_verified = ignore_verified.unwrap_or(true);
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -834,7 +834,7 @@ pub fn prune(
let group = BackupGroup::new(&backup_type, &backup_id);
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
@@ -959,7 +959,7 @@ pub fn prune_datastore(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
@@ -1003,7 +1003,7 @@ pub fn start_garbage_collection(
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let job = Job::new("garbage_collection", &store)
@@ -1039,7 +1039,7 @@ pub fn garbage_collection_status(
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<GarbageCollectionStatus, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let status = datastore.last_gc_status();
@@ -1115,7 +1115,7 @@ pub fn download_file(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1185,7 +1185,7 @@ pub fn download_file_decoded(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1299,7 +1299,7 @@ pub fn upload_backup_log(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(store, Some(Operation::Write))?;
let file_name = CLIENT_LOG_BLOB_NAME;
@@ -1376,7 +1376,7 @@ pub fn catalog(
filepath: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<ArchiveEntry>, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1446,7 +1446,7 @@ pub fn pxar_file_download(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1601,7 +1601,7 @@ pub fn get_group_notes(
backup_id: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_group = BackupGroup::new(backup_type, backup_id);
@@ -1643,7 +1643,7 @@ pub fn set_group_notes(
notes: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_group = BackupGroup::new(backup_type, backup_id);
@@ -1685,7 +1685,7 @@ pub fn get_notes(
backup_time: i64,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1736,7 +1736,7 @@ pub fn set_notes(
notes: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1779,7 +1779,7 @@ pub fn get_protection(
backup_time: i64,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<bool, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1824,7 +1824,7 @@ pub fn set_protection(
protected: bool,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1865,7 +1865,7 @@ pub fn set_backup_owner(
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let backup_group = BackupGroup::new(backup_type, backup_id);
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index eea62500..51c03987 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -15,7 +15,7 @@ use proxmox_router::{
use proxmox_schema::*;
use pbs_api_types::{
- Authid, VerifyState, SnapshotVerifyState,
+ Authid, Operation, VerifyState, SnapshotVerifyState,
BACKUP_ID_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_BACKUP, BACKUP_ARCHIVE_NAME_SCHEMA,
};
@@ -76,7 +76,7 @@ async move {
let user_info = CachedUserInfo::new()?;
user_info.check_privs(&auth_id, &["datastore", &store], PRIV_DATASTORE_BACKUP, false)?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let backup_type = required_string_param(¶m, "backup-type")?;
let backup_id = required_string_param(¶m, "backup-id")?;
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index 4280d922..c115b9aa 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -9,7 +9,7 @@ use proxmox_router::{ApiMethod, Router, RpcEnvironment, Permission};
use pbs_client::{HttpClient, BackupRepository};
use pbs_api_types::{
- Remote, Authid, SyncJobConfig,
+ Remote, Authid, Operation, SyncJobConfig,
DATASTORE_SCHEMA, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA,
PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_PRUNE, PRIV_REMOTE_READ,
};
@@ -46,7 +46,7 @@ pub async fn get_pull_parameters(
remote_store: &str,
) -> Result<(HttpClient, BackupRepository, Arc<DataStore>), Error> {
- let tgt_store = DataStore::lookup_datastore(store)?;
+ let tgt_store = DataStore::lookup_datastore(store, Some(Operation::Write))?;
let (remote_config, _digest) = pbs_config::remote::config()?;
let remote: Remote = remote_config.lookup("remote", remote)?;
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index f418f50f..6001a052 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -15,8 +15,8 @@ use proxmox_router::{
use proxmox_schema::{BooleanSchema, ObjectSchema};
use pbs_api_types::{
- Authid, DATASTORE_SCHEMA, BACKUP_TYPE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_ID_SCHEMA,
- CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP,
+ Authid, Operation, DATASTORE_SCHEMA, BACKUP_TYPE_SCHEMA, BACKUP_TIME_SCHEMA,
+ BACKUP_ID_SCHEMA, CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP,
BACKUP_ARCHIVE_NAME_SCHEMA,
};
use pbs_tools::fs::lock_dir_noblock_shared;
@@ -80,7 +80,7 @@ fn upgrade_to_backup_reader_protocol(
bail!("no permissions on /datastore/{}", store);
}
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let backup_type = required_string_param(¶m, "backup-type")?;
let backup_id = required_string_param(¶m, "backup-id")?;
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 7f50914b..ffbab000 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -14,7 +14,7 @@ use proxmox_router::{
use proxmox_router::list_subdirs_api_method;
use pbs_api_types::{
- Authid, DATASTORE_SCHEMA, RRDMode, RRDTimeFrame,
+ Authid, Operation, DATASTORE_SCHEMA, RRDMode, RRDTimeFrame,
PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
};
@@ -97,7 +97,7 @@ pub fn datastore_status(
continue;
}
- let datastore = match DataStore::lookup_datastore(&store) {
+ let datastore = match DataStore::lookup_datastore(&store, Some(Operation::Read)) {
Ok(datastore) => datastore,
Err(err) => {
list.push(json!({
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index a0578391..46b59658 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -9,7 +9,7 @@ use proxmox_router::{Permission, Router, RpcEnvironment, RpcEnvironmentType};
use proxmox_schema::api;
use pbs_api_types::{
- Authid, Userid, TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, MediaPoolConfig,
+ Authid, Userid, Operation, TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, MediaPoolConfig,
UPID_SCHEMA, JOB_ID_SCHEMA, PRIV_DATASTORE_READ, PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE,
};
@@ -167,7 +167,7 @@ pub fn do_tape_backup_job(
let worker_type = job.jobtype().to_string();
- let datastore = DataStore::lookup_datastore(&setup.store)?;
+ let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Write))?;
let (config, _digest) = pbs_config::media_pool::config()?;
let pool_config: MediaPoolConfig = config.lookup("pool", &setup.pool)?;
@@ -348,7 +348,7 @@ pub fn backup(
&setup.drive,
)?;
- let datastore = DataStore::lookup_datastore(&setup.store)?;
+ let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Write))?;
let (config, _digest) = pbs_config::media_pool::config()?;
let pool_config: MediaPoolConfig = config.lookup("pool", &setup.pool)?;
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 40a2414f..ad7c7ce9 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -16,7 +16,7 @@ use proxmox_section_config::SectionConfigData;
use proxmox_uuid::Uuid;
use pbs_api_types::{
- Authid, Userid, CryptMode,
+ Authid, Operation, Userid, CryptMode,
DATASTORE_MAP_ARRAY_SCHEMA, DATASTORE_MAP_LIST_SCHEMA, DRIVE_NAME_SCHEMA,
UPID_SCHEMA, TAPE_RESTORE_SNAPSHOT_SCHEMA,
PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_TAPE_READ,
@@ -93,10 +93,10 @@ impl TryFrom<String> for DataStoreMap {
if let Some(index) = store.find('=') {
let mut target = store.split_off(index);
target.remove(0); // remove '='
- let datastore = DataStore::lookup_datastore(&target)?;
+ let datastore = DataStore::lookup_datastore(&target, Some(Operation::Read))?;
map.insert(store, datastore);
} else if default.is_none() {
- default = Some(DataStore::lookup_datastore(&store)?);
+ default = Some(DataStore::lookup_datastore(&store, Some(Operation::Read))?);
} else {
bail!("multiple default stores given");
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index a3bd4cfa..af471ef6 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -49,8 +49,8 @@ use proxmox_systemd::time::{compute_next_event, parse_calendar_event};
use pbs_tools::logrotate::LogRotate;
use pbs_api_types::{
- Authid, TapeBackupJobConfig, VerificationJobConfig, SyncJobConfig, DataStoreConfig,
- PruneOptions,
+ Authid, Operation, TapeBackupJobConfig, VerificationJobConfig,
+ SyncJobConfig, DataStoreConfig, PruneOptions,
};
use proxmox_rest_server::daemon;
@@ -536,7 +536,7 @@ async fn schedule_datastore_garbage_collection() {
};
for (store, (_, store_config)) in config.sections {
- let datastore = match DataStore::lookup_datastore(&store) {
+ let datastore = match DataStore::lookup_datastore(&store, Some(Operation::Write)) {
Ok(datastore) => datastore,
Err(err) => {
eprintln!("lookup_datastore failed - {}", err);
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 0fc68118..f3a5848f 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -5,7 +5,7 @@ use anyhow::Error;
use pbs_datastore::backup_info::BackupInfo;
use pbs_datastore::prune::compute_prune_info;
use pbs_datastore::DataStore;
-use pbs_api_types::{Authid, PRIV_DATASTORE_MODIFY, PruneOptions};
+use pbs_api_types::{Authid, Operation, PRIV_DATASTORE_MODIFY, PruneOptions};
use pbs_config::CachedUserInfo;
use pbs_tools::{task_log, task_warn};
use proxmox_rest_server::WorkerTask;
@@ -96,7 +96,7 @@ pub fn do_prune_job(
auth_id: &Authid,
schedule: Option<String>,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let worker_type = job.jobtype().to_string();
let auth_id = auth_id.clone();
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index 6aba97c9..b0a29340 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -1,7 +1,7 @@
use anyhow::{format_err, Error};
use pbs_tools::task_log;
-use pbs_api_types::{Authid, VerificationJobConfig};
+use pbs_api_types::{Authid, Operation, VerificationJobConfig};
use proxmox_rest_server::WorkerTask;
use pbs_datastore::DataStore;
@@ -22,7 +22,7 @@ pub fn do_verification_job(
to_stdout: bool,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&verification_job.store)?;
+ let datastore = DataStore::lookup_datastore(&verification_job.store, Some(Operation::Read))?;
let outdated_after = verification_job.outdated_after;
let ignore_verified_snapshots = verification_job.ignore_verified.unwrap_or(true);
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH v4 proxmox-backup 3/6] pbs-datastore: add active operations tracking
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 1/6] pbs-api-types: add maintenance type Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 2/6] pbs-datastore: add check for maintenance in lookup Hannes Laimer
@ 2021-11-12 12:30 ` Hannes Laimer
2021-11-19 15:21 ` Dominik Csapak
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 4/6] api2: make maintenance_type updatable Hannes Laimer
` (3 subsequent siblings)
6 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2021-11-12 12:30 UTC (permalink / raw)
To: pbs-devel
Saves the currently active read/write operation counts in a file. The
file is updated whenever a reference returned by lookup_datastore is
dropped and whenever a reference is returned by lookup_datastore. The
files are locked before every access, there is one file per datastore.
---
v3->v4: implement Clone for the DataStore wrapper struct. So clones are
tracked correctly. On every update of the file, the lines of inactive
processes are removed. A line looks like this:
<pid> <read> <write>
So, PID has <read>/<write> active read/write operations. This is needed
because if we just had one number for read/write and some process would
die, we are not able to find out how many operations were ended with
it. Like this we only consider lines that have a PID that belongs to an
active process.
pbs-api-types/src/maintenance.rs | 1 +
pbs-datastore/Cargo.toml | 1 +
pbs-datastore/src/datastore.rs | 166 +++++++++++++++++++++++++------
pbs-datastore/src/lib.rs | 3 +
src/bin/proxmox-backup-api.rs | 1 +
src/server/mod.rs | 16 ++-
6 files changed, 158 insertions(+), 30 deletions(-)
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index f816b279..98e3ec62 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -25,6 +25,7 @@ pub enum MaintenanceType {
Offline(String),
}
+#[derive(Copy ,Clone)]
/// Operation requirments, used when checking for maintenance mode.
pub enum Operation {
Read,
diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml
index 01c5ee00..2063c1bb 100644
--- a/pbs-datastore/Cargo.toml
+++ b/pbs-datastore/Cargo.toml
@@ -34,5 +34,6 @@ proxmox-time = "1"
proxmox-uuid = "1"
pbs-api-types = { path = "../pbs-api-types" }
+pbs-buildcfg = { path = "../pbs-buildcfg" }
pbs-tools = { path = "../pbs-tools" }
pbs-config = { path = "../pbs-config" }
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 064fd273..d8c2f3f5 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -9,6 +9,7 @@ use std::time::Duration;
use anyhow::{bail, format_err, Error};
use lazy_static::lazy_static;
+use proxmox::sys::linux::procfs;
use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, MaintenanceType, Operation};
@@ -31,7 +32,7 @@ use crate::manifest::{
};
lazy_static! {
- static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStore>>> = Mutex::new(HashMap::new());
+ static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStoreImpl>>> = Mutex::new(HashMap::new());
}
/// checks if auth_id is owner, or, if owner is a token, if
@@ -52,13 +53,113 @@ pub fn check_backup_owner(
///
/// A Datastore can store severals backups, and provides the
/// management interface for backup.
-pub struct DataStore {
+pub struct DataStoreImpl {
chunk_store: Arc<ChunkStore>,
gc_mutex: Mutex<()>,
last_gc_status: Mutex<GarbageCollectionStatus>,
verify_new: bool,
}
+pub struct DataStore {
+ inner: Arc<DataStoreImpl>,
+ operation: Option<Operation>,
+}
+
+impl Clone for DataStore {
+ fn clone(&self) -> Self {
+ if let Some(operation) = self.operation.clone() {
+ if let Err(e) = update_active_operations(self.name(), operation, -1) {
+ eprintln!("could not update active operations - {}", e);
+ }
+ }
+ DataStore {
+ inner: self.inner.clone(),
+ operation: self.operation.clone(),
+ }
+ }
+}
+
+fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> {
+ let mut path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
+ let mut lock_path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
+ path.push(name);
+ lock_path.push(format!(".{}.lock", name));
+
+ let user = pbs_config::backup_user()?;
+ let options = proxmox::tools::fs::CreateOptions::new()
+ .group(user.gid)
+ .owner(user.uid)
+ .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
+
+ let timeout = std::time::Duration::new(10, 0);
+ proxmox::tools::fs::open_file_locked(&lock_path, timeout, true, options.clone())?;
+
+ let pid = std::process::id() as i64;
+ proxmox::tools::fs::replace_file(
+ &path,
+ match proxmox::tools::fs::file_get_optional_contents(&path) {
+ Ok(Some(data)) => {
+ let mut updated = false;
+ let new_data = String::from_utf8(data)
+ .unwrap_or(String::new())
+ .lines()
+ .into_iter()
+ .fold(String::new(), |xs, line| {
+ let split = line.split(" ").collect::<Vec<&str>>();
+ match split[0].parse::<i32>() {
+ Ok(line_pid) if line_pid as i64 == pid && split.len() == 3 => {
+ let stat = (
+ split[1].parse::<i64>().unwrap_or(0),
+ split[2].parse::<i64>().unwrap_or(0),
+ );
+ updated = true;
+ match (operation, stat) {
+ (Operation::Write, (r, w)) => {
+ format!("{}\n{} {} {}", xs, pid, r, w + count)
+ }
+ (Operation::Read, (r, w)) => {
+ format!("{}\n{} {} {}", xs, pid, r + count, w)
+ }
+ }
+ }
+ Ok(line_pid)
+ if procfs::check_process_running(line_pid).is_some()
+ && split.len() == 3 =>
+ {
+ format!("{}\n{}", xs, line)
+ }
+ _ => xs,
+ }
+ });
+ match operation {
+ Operation::Read if !updated => format!("{}\n{} {} {}", new_data, pid, 1, 0),
+ Operation::Write if !updated => format!("{}\n{} {} {}", new_data, pid, 0, 1),
+ _ => new_data,
+ }
+ }
+ _ => match operation {
+ Operation::Read => format!("{} {} {}", pid, 1, 0),
+ Operation::Write => format!("{} {} {}", pid, 0, 1),
+ },
+ }
+ .as_bytes(),
+ options,
+ false,
+ )?;
+
+ Ok(())
+}
+
+impl Drop for DataStore {
+ fn drop(&mut self) {
+ if let Some(operation) = self.operation.clone() {
+ if let Err(e) = update_active_operations(self.name(), operation, -1) {
+ eprintln!("could not update active operations - {}", e);
+ }
+ }
+ }
+}
+
impl DataStore {
pub fn lookup_datastore(
name: &str,
@@ -73,6 +174,7 @@ impl DataStore {
| (Some(MaintenanceType::Offline(message)), Some(_)) => {
bail!("Datastore '{}' is in maintenance mode: {}", name, message);
},
+ (_, Some(operation)) => update_active_operations(name, operation, 1)?,
_ => {}
}
@@ -83,7 +185,10 @@ impl DataStore {
if datastore.chunk_store.base() == path &&
datastore.verify_new == config.verify_new.unwrap_or(false)
{
- return Ok(datastore.clone());
+ return Ok(Arc::new(Self {
+ inner: datastore.clone(),
+ operation,
+ }))
}
}
@@ -92,7 +197,10 @@ impl DataStore {
let datastore = Arc::new(datastore);
map.insert(name.to_string(), datastore.clone());
- Ok(datastore)
+ Ok(Arc::new(Self {
+ inner: datastore,
+ operation,
+ }))
}
/// removes all datastores that are not configured anymore
@@ -107,7 +215,7 @@ impl DataStore {
Ok(())
}
- fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<Self, Error> {
+ fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<DataStoreImpl, Error> {
let chunk_store = ChunkStore::open(store_name, path)?;
let mut gc_status_path = chunk_store.base_path();
@@ -125,7 +233,7 @@ impl DataStore {
GarbageCollectionStatus::default()
};
- Ok(Self {
+ Ok(DataStoreImpl {
chunk_store: Arc::new(chunk_store),
gc_mutex: Mutex::new(()),
last_gc_status: Mutex::new(gc_status),
@@ -139,19 +247,19 @@ impl DataStore {
impl Iterator<Item = (Result<pbs_tools::fs::ReadDirEntry, Error>, usize, bool)>,
Error
> {
- self.chunk_store.get_chunk_iterator()
+ self.inner.chunk_store.get_chunk_iterator()
}
pub fn create_fixed_writer<P: AsRef<Path>>(&self, filename: P, size: usize, chunk_size: usize) -> Result<FixedIndexWriter, Error> {
- let index = FixedIndexWriter::create(self.chunk_store.clone(), filename.as_ref(), size, chunk_size)?;
+ let index = FixedIndexWriter::create(self.inner.chunk_store.clone(), filename.as_ref(), size, chunk_size)?;
Ok(index)
}
pub fn open_fixed_reader<P: AsRef<Path>>(&self, filename: P) -> Result<FixedIndexReader, Error> {
- let full_path = self.chunk_store.relative_path(filename.as_ref());
+ let full_path = self.inner.chunk_store.relative_path(filename.as_ref());
let index = FixedIndexReader::open(&full_path)?;
@@ -163,14 +271,14 @@ impl DataStore {
) -> Result<DynamicIndexWriter, Error> {
let index = DynamicIndexWriter::create(
- self.chunk_store.clone(), filename.as_ref())?;
+ self.inner.chunk_store.clone(), filename.as_ref())?;
Ok(index)
}
pub fn open_dynamic_reader<P: AsRef<Path>>(&self, filename: P) -> Result<DynamicIndexReader, Error> {
- let full_path = self.chunk_store.relative_path(filename.as_ref());
+ let full_path = self.inner.chunk_store.relative_path(filename.as_ref());
let index = DynamicIndexReader::open(&full_path)?;
@@ -220,11 +328,11 @@ impl DataStore {
}
pub fn name(&self) -> &str {
- self.chunk_store.name()
+ self.inner.chunk_store.name()
}
pub fn base_path(&self) -> PathBuf {
- self.chunk_store.base_path()
+ self.inner.chunk_store.base_path()
}
/// Cleanup a backup directory
@@ -530,7 +638,7 @@ impl DataStore {
worker.check_abort()?;
worker.fail_on_shutdown()?;
let digest = index.index_digest(pos).unwrap();
- if !self.chunk_store.cond_touch_chunk(digest, false)? {
+ if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
task_warn!(
worker,
"warning: unable to access non-existent chunk {}, required by {:?}",
@@ -546,7 +654,7 @@ impl DataStore {
let mut bad_path = PathBuf::new();
bad_path.push(self.chunk_path(digest).0);
bad_path.set_extension(bad_ext);
- self.chunk_store.cond_touch_path(&bad_path, false)?;
+ self.inner.chunk_store.cond_touch_path(&bad_path, false)?;
}
}
}
@@ -626,24 +734,24 @@ impl DataStore {
}
pub fn last_gc_status(&self) -> GarbageCollectionStatus {
- self.last_gc_status.lock().unwrap().clone()
+ self.inner.last_gc_status.lock().unwrap().clone()
}
pub fn garbage_collection_running(&self) -> bool {
- !matches!(self.gc_mutex.try_lock(), Ok(_))
+ !matches!(self.inner.gc_mutex.try_lock(), Ok(_))
}
pub fn garbage_collection(&self, worker: &dyn WorkerTaskContext, upid: &UPID) -> Result<(), Error> {
- if let Ok(ref mut _mutex) = self.gc_mutex.try_lock() {
+ if let Ok(ref mut _mutex) = self.inner.gc_mutex.try_lock() {
// avoids that we run GC if an old daemon process has still a
// running backup writer, which is not save as we have no "oldest
// writer" information and thus no safe atime cutoff
- let _exclusive_lock = self.chunk_store.try_exclusive_lock()?;
+ let _exclusive_lock = self.inner.chunk_store.try_exclusive_lock()?;
let phase1_start_time = proxmox_time::epoch_i64();
- let oldest_writer = self.chunk_store.oldest_writer().unwrap_or(phase1_start_time);
+ let oldest_writer = self.inner.chunk_store.oldest_writer().unwrap_or(phase1_start_time);
let mut gc_status = GarbageCollectionStatus::default();
gc_status.upid = Some(upid.to_string());
@@ -653,7 +761,7 @@ impl DataStore {
self.mark_used_chunks(&mut gc_status, worker)?;
task_log!(worker, "Start GC phase2 (sweep unused chunks)");
- self.chunk_store.sweep_unused_chunks(
+ self.inner.chunk_store.sweep_unused_chunks(
oldest_writer,
phase1_start_time,
&mut gc_status,
@@ -730,7 +838,7 @@ impl DataStore {
let _ = replace_file(path, serialized.as_bytes(), options, false);
}
- *self.last_gc_status.lock().unwrap() = gc_status;
+ *self.inner.last_gc_status.lock().unwrap() = gc_status;
} else {
bail!("Start GC failed - (already running/locked)");
@@ -740,15 +848,15 @@ impl DataStore {
}
pub fn try_shared_chunk_store_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
- self.chunk_store.try_shared_lock()
+ self.inner.chunk_store.try_shared_lock()
}
pub fn chunk_path(&self, digest:&[u8; 32]) -> (PathBuf, String) {
- self.chunk_store.chunk_path(digest)
+ self.inner.chunk_store.chunk_path(digest)
}
pub fn cond_touch_chunk(&self, digest: &[u8; 32], fail_if_not_exist: bool) -> Result<bool, Error> {
- self.chunk_store.cond_touch_chunk(digest, fail_if_not_exist)
+ self.inner.chunk_store.cond_touch_chunk(digest, fail_if_not_exist)
}
pub fn insert_chunk(
@@ -756,7 +864,7 @@ impl DataStore {
chunk: &DataBlob,
digest: &[u8; 32],
) -> Result<(bool, u64), Error> {
- self.chunk_store.insert_chunk(chunk, digest)
+ self.inner.chunk_store.insert_chunk(chunk, digest)
}
pub fn load_blob(&self, backup_dir: &BackupDir, filename: &str) -> Result<DataBlob, Error> {
@@ -772,13 +880,13 @@ impl DataStore {
pub fn stat_chunk(&self, digest: &[u8; 32]) -> Result<std::fs::Metadata, Error> {
- let (chunk_path, _digest_str) = self.chunk_store.chunk_path(digest);
+ let (chunk_path, _digest_str) = self.inner.chunk_store.chunk_path(digest);
std::fs::metadata(chunk_path).map_err(Error::from)
}
pub fn load_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
- let (chunk_path, digest_str) = self.chunk_store.chunk_path(digest);
+ let (chunk_path, digest_str) = self.inner.chunk_store.chunk_path(digest);
proxmox_lang::try_block!({
let mut file = std::fs::File::open(&chunk_path)?;
@@ -892,7 +1000,7 @@ impl DataStore {
}
pub fn verify_new(&self) -> bool {
- self.verify_new
+ self.inner.verify_new
}
/// returns a list of chunks sorted by their inode number on disk
diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
index d50a64a5..159642fd 100644
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -145,6 +145,9 @@
// Note: .pcat1 => Proxmox Catalog Format version 1
pub const CATALOG_NAME: &str = "catalog.pcat1.didx";
+/// Directory path where active operations counters are saved.
+pub const ACTIVE_OPERATIONS_DIR: &str = concat!(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!(), "/active-operations");
+
#[macro_export]
macro_rules! PROXMOX_BACKUP_PROTOCOL_ID_V1 {
() => {
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index 9eb20269..aa60d316 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -73,6 +73,7 @@ async fn run() -> Result<(), Error> {
proxmox_backup::server::create_run_dir()?;
proxmox_backup::server::create_state_dir()?;
+ proxmox_backup::server::create_active_operations_dir()?;
proxmox_backup::server::jobstate::create_jobstate_dir()?;
proxmox_backup::tape::create_tape_status_dir()?;
proxmox_backup::tape::create_drive_state_dir()?;
diff --git a/src/server/mod.rs b/src/server/mod.rs
index deeb3398..ffb26f0e 100644
--- a/src/server/mod.rs
+++ b/src/server/mod.rs
@@ -4,7 +4,7 @@
//! services. We want async IO, so this is built on top of
//! tokio/hyper.
-use anyhow::Error;
+use anyhow::{format_err, Error};
use serde_json::Value;
use proxmox::tools::fs::{create_path, CreateOptions};
@@ -71,3 +71,17 @@ pub fn create_state_dir() -> Result<(), Error> {
create_path(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR_M!(), None, Some(opts))?;
Ok(())
}
+
+/// Create active operations dir with correct permission.
+pub fn create_active_operations_dir() -> Result<(), Error> {
+ let backup_user = pbs_config::backup_user()?;
+ let mode = nix::sys::stat::Mode::from_bits_truncate(0o0750);
+ let options = CreateOptions::new()
+ .perm(mode)
+ .owner(backup_user.uid)
+ .group(backup_user.gid);
+
+ create_path(pbs_datastore::ACTIVE_OPERATIONS_DIR, None, Some(options))
+ .map_err(|err: Error| format_err!("unable to create active operations dir - {}", err))?;
+ Ok(())
+}
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [pbs-devel] [PATCH v4 proxmox-backup 3/6] pbs-datastore: add active operations tracking
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 3/6] pbs-datastore: add active operations tracking Hannes Laimer
@ 2021-11-19 15:21 ` Dominik Csapak
0 siblings, 0 replies; 9+ messages in thread
From: Dominik Csapak @ 2021-11-19 15:21 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comments inline
On 11/12/21 13:30, Hannes Laimer wrote:
> Saves the currently active read/write operation counts in a file. The
> file is updated whenever a reference returned by lookup_datastore is
> dropped and whenever a reference is returned by lookup_datastore. The
> files are locked before every access, there is one file per datastore.
> ---
> v3->v4: implement Clone for the DataStore wrapper struct. So clones are
> tracked correctly. On every update of the file, the lines of inactive
> processes are removed. A line looks like this:
> <pid> <read> <write>
> So, PID has <read>/<write> active read/write operations. This is needed
> because if we just had one number for read/write and some process would
> die, we are not able to find out how many operations were ended with
> it. Like this we only consider lines that have a PID that belongs to an
> active process.
>
> pbs-api-types/src/maintenance.rs | 1 +
> pbs-datastore/Cargo.toml | 1 +
> pbs-datastore/src/datastore.rs | 166 +++++++++++++++++++++++++------
> pbs-datastore/src/lib.rs | 3 +
> src/bin/proxmox-backup-api.rs | 1 +
> src/server/mod.rs | 16 ++-
> 6 files changed, 158 insertions(+), 30 deletions(-)
>
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> index f816b279..98e3ec62 100644
> --- a/pbs-api-types/src/maintenance.rs
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -25,6 +25,7 @@ pub enum MaintenanceType {
> Offline(String),
> }
>
> +#[derive(Copy ,Clone)]
> /// Operation requirments, used when checking for maintenance mode.
> pub enum Operation {
> Read,
> diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml
> index 01c5ee00..2063c1bb 100644
> --- a/pbs-datastore/Cargo.toml
> +++ b/pbs-datastore/Cargo.toml
> @@ -34,5 +34,6 @@ proxmox-time = "1"
> proxmox-uuid = "1"
>
> pbs-api-types = { path = "../pbs-api-types" }
> +pbs-buildcfg = { path = "../pbs-buildcfg" }
> pbs-tools = { path = "../pbs-tools" }
> pbs-config = { path = "../pbs-config" }
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 064fd273..d8c2f3f5 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -9,6 +9,7 @@ use std::time::Duration;
> use anyhow::{bail, format_err, Error};
> use lazy_static::lazy_static;
>
> +use proxmox::sys::linux::procfs;
> use proxmox::tools::fs::{replace_file, file_read_optional_string, CreateOptions};
>
> use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, MaintenanceType, Operation};
> @@ -31,7 +32,7 @@ use crate::manifest::{
> };
>
> lazy_static! {
> - static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStore>>> = Mutex::new(HashMap::new());
> + static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStoreImpl>>> = Mutex::new(HashMap::new());
> }
>
> /// checks if auth_id is owner, or, if owner is a token, if
> @@ -52,13 +53,113 @@ pub fn check_backup_owner(
> ///
> /// A Datastore can store severals backups, and provides the
> /// management interface for backup.
> -pub struct DataStore {
> +pub struct DataStoreImpl {
> chunk_store: Arc<ChunkStore>,
> gc_mutex: Mutex<()>,
> last_gc_status: Mutex<GarbageCollectionStatus>,
> verify_new: bool,
> }
>
> +pub struct DataStore {
> + inner: Arc<DataStoreImpl>,
> + operation: Option<Operation>,
> +}
> +
> +impl Clone for DataStore {
> + fn clone(&self) -> Self {
> + if let Some(operation) = self.operation.clone() {
> + if let Err(e) = update_active_operations(self.name(), operation, -1) {
> + eprintln!("could not update active operations - {}", e);
> + }
> + }
> + DataStore {
> + inner: self.inner.clone(),
> + operation: self.operation.clone(),
> + }
> + }
> +}
> +
> +fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> {
> + let mut path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
> + let mut lock_path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
> + path.push(name);
> + lock_path.push(format!(".{}.lock", name));
> +
> + let user = pbs_config::backup_user()?;
> + let options = proxmox::tools::fs::CreateOptions::new()
> + .group(user.gid)
> + .owner(user.uid)
> + .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
> +
> + let timeout = std::time::Duration::new(10, 0);
> + proxmox::tools::fs::open_file_locked(&lock_path, timeout, true, options.clone())?;
> +
> + let pid = std::process::id() as i64;
> + proxmox::tools::fs::replace_file(
> + &path,
> + match proxmox::tools::fs::file_get_optional_contents(&path) {
you could do that outside of the replace_file call
removes a level of indentation
> + Ok(Some(data)) => {
> + let mut updated = false;
> + let new_data = String::from_utf8(data)
we already have: file_read_optional_string
so no need to convert it ourselves
removes of at least one level of intendation
> + .unwrap_or(String::new())
> + .lines()
> + .into_iter()
> + .fold(String::new(), |xs, line| {
any special reason to use fold with the permanent allocation (format!())
instead of simply using a loop and appending to a string?
> + let split = line.split(" ").collect::<Vec<&str>>();
> + match split[0].parse::<i32>() {
> + Ok(line_pid) if line_pid as i64 == pid && split.len() == 3 => {
> + let stat = (
> + split[1].parse::<i64>().unwrap_or(0),
> + split[2].parse::<i64>().unwrap_or(0),
> + );
> + updated = true;
> + match (operation, stat) {
> + (Operation::Write, (r, w)) => {
> + format!("{}\n{} {} {}", xs, pid, r, w + count)
> + }
> + (Operation::Read, (r, w)) => {
> + format!("{}\n{} {} {}", xs, pid, r + count, w)
> + }
> + }
> + }
> + Ok(line_pid)
> + if procfs::check_process_running(line_pid).is_some()
> + && split.len() == 3 =>
> + {
> + format!("{}\n{}", xs, line)
> + }
> + _ => xs,
imho, the line parsing/printing should be their own function, that
way one can more easily reason about the behaviour...
> + }
> + });
> + match operation {
> + Operation::Read if !updated => format!("{}\n{} {} {}", new_data, pid, 1, 0),
> + Operation::Write if !updated => format!("{}\n{} {} {}", new_data, pid, 0, 1),
> + _ => new_data,
> + }
> + }
> + _ => match operation {
> + Operation::Read => format!("{} {} {}", pid, 1, 0),
> + Operation::Write => format!("{} {} {}", pid, 0, 1),
> + },
> + }
> + .as_bytes(),
> + options,
> + false,
> + )?;
> +
> + Ok(())
> +}
> +
> +impl Drop for DataStore {
> + fn drop(&mut self) {
> + if let Some(operation) = self.operation.clone() {
> + if let Err(e) = update_active_operations(self.name(), operation, -1) {
> + eprintln!("could not update active operations - {}", e);
> + }
> + }
> + }
> +}
> +
> impl DataStore {
> pub fn lookup_datastore(
> name: &str,
> @@ -73,6 +174,7 @@ impl DataStore {
> | (Some(MaintenanceType::Offline(message)), Some(_)) => {
> bail!("Datastore '{}' is in maintenance mode: {}", name, message);
> },
> + (_, Some(operation)) => update_active_operations(name, operation, 1)?,
> _ => {}
> }
>
> @@ -83,7 +185,10 @@ impl DataStore {
> if datastore.chunk_store.base() == path &&
> datastore.verify_new == config.verify_new.unwrap_or(false)
> {
> - return Ok(datastore.clone());
> + return Ok(Arc::new(Self {
> + inner: datastore.clone(),
> + operation,
> + }))
> }
> }
>
> @@ -92,7 +197,10 @@ impl DataStore {
> let datastore = Arc::new(datastore);
> map.insert(name.to_string(), datastore.clone());
>
> - Ok(datastore)
> + Ok(Arc::new(Self {
> + inner: datastore,
> + operation,
> + }))
> }
>
> /// removes all datastores that are not configured anymore
> @@ -107,7 +215,7 @@ impl DataStore {
> Ok(())
> }
>
> - fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<Self, Error> {
> + fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<DataStoreImpl, Error> {
> let chunk_store = ChunkStore::open(store_name, path)?;
>
> let mut gc_status_path = chunk_store.base_path();
> @@ -125,7 +233,7 @@ impl DataStore {
> GarbageCollectionStatus::default()
> };
>
> - Ok(Self {
> + Ok(DataStoreImpl {
> chunk_store: Arc::new(chunk_store),
> gc_mutex: Mutex::new(()),
> last_gc_status: Mutex::new(gc_status),
> @@ -139,19 +247,19 @@ impl DataStore {
> impl Iterator<Item = (Result<pbs_tools::fs::ReadDirEntry, Error>, usize, bool)>,
> Error
> > {
> - self.chunk_store.get_chunk_iterator()
> + self.inner.chunk_store.get_chunk_iterator()
> }
>
> pub fn create_fixed_writer<P: AsRef<Path>>(&self, filename: P, size: usize, chunk_size: usize) -> Result<FixedIndexWriter, Error> {
>
> - let index = FixedIndexWriter::create(self.chunk_store.clone(), filename.as_ref(), size, chunk_size)?;
> + let index = FixedIndexWriter::create(self.inner.chunk_store.clone(), filename.as_ref(), size, chunk_size)?;
>
> Ok(index)
> }
>
> pub fn open_fixed_reader<P: AsRef<Path>>(&self, filename: P) -> Result<FixedIndexReader, Error> {
>
> - let full_path = self.chunk_store.relative_path(filename.as_ref());
> + let full_path = self.inner.chunk_store.relative_path(filename.as_ref());
>
> let index = FixedIndexReader::open(&full_path)?;
>
> @@ -163,14 +271,14 @@ impl DataStore {
> ) -> Result<DynamicIndexWriter, Error> {
>
> let index = DynamicIndexWriter::create(
> - self.chunk_store.clone(), filename.as_ref())?;
> + self.inner.chunk_store.clone(), filename.as_ref())?;
>
> Ok(index)
> }
>
> pub fn open_dynamic_reader<P: AsRef<Path>>(&self, filename: P) -> Result<DynamicIndexReader, Error> {
>
> - let full_path = self.chunk_store.relative_path(filename.as_ref());
> + let full_path = self.inner.chunk_store.relative_path(filename.as_ref());
>
> let index = DynamicIndexReader::open(&full_path)?;
>
> @@ -220,11 +328,11 @@ impl DataStore {
> }
>
> pub fn name(&self) -> &str {
> - self.chunk_store.name()
> + self.inner.chunk_store.name()
> }
>
> pub fn base_path(&self) -> PathBuf {
> - self.chunk_store.base_path()
> + self.inner.chunk_store.base_path()
> }
>
> /// Cleanup a backup directory
> @@ -530,7 +638,7 @@ impl DataStore {
> worker.check_abort()?;
> worker.fail_on_shutdown()?;
> let digest = index.index_digest(pos).unwrap();
> - if !self.chunk_store.cond_touch_chunk(digest, false)? {
> + if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
> task_warn!(
> worker,
> "warning: unable to access non-existent chunk {}, required by {:?}",
> @@ -546,7 +654,7 @@ impl DataStore {
> let mut bad_path = PathBuf::new();
> bad_path.push(self.chunk_path(digest).0);
> bad_path.set_extension(bad_ext);
> - self.chunk_store.cond_touch_path(&bad_path, false)?;
> + self.inner.chunk_store.cond_touch_path(&bad_path, false)?;
> }
> }
> }
> @@ -626,24 +734,24 @@ impl DataStore {
> }
>
> pub fn last_gc_status(&self) -> GarbageCollectionStatus {
> - self.last_gc_status.lock().unwrap().clone()
> + self.inner.last_gc_status.lock().unwrap().clone()
> }
>
> pub fn garbage_collection_running(&self) -> bool {
> - !matches!(self.gc_mutex.try_lock(), Ok(_))
> + !matches!(self.inner.gc_mutex.try_lock(), Ok(_))
> }
>
> pub fn garbage_collection(&self, worker: &dyn WorkerTaskContext, upid: &UPID) -> Result<(), Error> {
>
> - if let Ok(ref mut _mutex) = self.gc_mutex.try_lock() {
> + if let Ok(ref mut _mutex) = self.inner.gc_mutex.try_lock() {
>
> // avoids that we run GC if an old daemon process has still a
> // running backup writer, which is not save as we have no "oldest
> // writer" information and thus no safe atime cutoff
> - let _exclusive_lock = self.chunk_store.try_exclusive_lock()?;
> + let _exclusive_lock = self.inner.chunk_store.try_exclusive_lock()?;
>
> let phase1_start_time = proxmox_time::epoch_i64();
> - let oldest_writer = self.chunk_store.oldest_writer().unwrap_or(phase1_start_time);
> + let oldest_writer = self.inner.chunk_store.oldest_writer().unwrap_or(phase1_start_time);
>
> let mut gc_status = GarbageCollectionStatus::default();
> gc_status.upid = Some(upid.to_string());
> @@ -653,7 +761,7 @@ impl DataStore {
> self.mark_used_chunks(&mut gc_status, worker)?;
>
> task_log!(worker, "Start GC phase2 (sweep unused chunks)");
> - self.chunk_store.sweep_unused_chunks(
> + self.inner.chunk_store.sweep_unused_chunks(
> oldest_writer,
> phase1_start_time,
> &mut gc_status,
> @@ -730,7 +838,7 @@ impl DataStore {
> let _ = replace_file(path, serialized.as_bytes(), options, false);
> }
>
> - *self.last_gc_status.lock().unwrap() = gc_status;
> + *self.inner.last_gc_status.lock().unwrap() = gc_status;
>
> } else {
> bail!("Start GC failed - (already running/locked)");
> @@ -740,15 +848,15 @@ impl DataStore {
> }
>
> pub fn try_shared_chunk_store_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
> - self.chunk_store.try_shared_lock()
> + self.inner.chunk_store.try_shared_lock()
> }
>
> pub fn chunk_path(&self, digest:&[u8; 32]) -> (PathBuf, String) {
> - self.chunk_store.chunk_path(digest)
> + self.inner.chunk_store.chunk_path(digest)
> }
>
> pub fn cond_touch_chunk(&self, digest: &[u8; 32], fail_if_not_exist: bool) -> Result<bool, Error> {
> - self.chunk_store.cond_touch_chunk(digest, fail_if_not_exist)
> + self.inner.chunk_store.cond_touch_chunk(digest, fail_if_not_exist)
> }
>
> pub fn insert_chunk(
> @@ -756,7 +864,7 @@ impl DataStore {
> chunk: &DataBlob,
> digest: &[u8; 32],
> ) -> Result<(bool, u64), Error> {
> - self.chunk_store.insert_chunk(chunk, digest)
> + self.inner.chunk_store.insert_chunk(chunk, digest)
> }
>
> pub fn load_blob(&self, backup_dir: &BackupDir, filename: &str) -> Result<DataBlob, Error> {
> @@ -772,13 +880,13 @@ impl DataStore {
>
>
> pub fn stat_chunk(&self, digest: &[u8; 32]) -> Result<std::fs::Metadata, Error> {
> - let (chunk_path, _digest_str) = self.chunk_store.chunk_path(digest);
> + let (chunk_path, _digest_str) = self.inner.chunk_store.chunk_path(digest);
> std::fs::metadata(chunk_path).map_err(Error::from)
> }
>
> pub fn load_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
>
> - let (chunk_path, digest_str) = self.chunk_store.chunk_path(digest);
> + let (chunk_path, digest_str) = self.inner.chunk_store.chunk_path(digest);
>
> proxmox_lang::try_block!({
> let mut file = std::fs::File::open(&chunk_path)?;
> @@ -892,7 +1000,7 @@ impl DataStore {
> }
>
> pub fn verify_new(&self) -> bool {
> - self.verify_new
> + self.inner.verify_new
> }
>
> /// returns a list of chunks sorted by their inode number on disk
> diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
> index d50a64a5..159642fd 100644
> --- a/pbs-datastore/src/lib.rs
> +++ b/pbs-datastore/src/lib.rs
> @@ -145,6 +145,9 @@
> // Note: .pcat1 => Proxmox Catalog Format version 1
> pub const CATALOG_NAME: &str = "catalog.pcat1.didx";
>
> +/// Directory path where active operations counters are saved.
> +pub const ACTIVE_OPERATIONS_DIR: &str = concat!(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!(), "/active-operations");
> +
> #[macro_export]
> macro_rules! PROXMOX_BACKUP_PROTOCOL_ID_V1 {
> () => {
> diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
> index 9eb20269..aa60d316 100644
> --- a/src/bin/proxmox-backup-api.rs
> +++ b/src/bin/proxmox-backup-api.rs
> @@ -73,6 +73,7 @@ async fn run() -> Result<(), Error> {
>
> proxmox_backup::server::create_run_dir()?;
> proxmox_backup::server::create_state_dir()?;
> + proxmox_backup::server::create_active_operations_dir()?;
> proxmox_backup::server::jobstate::create_jobstate_dir()?;
> proxmox_backup::tape::create_tape_status_dir()?;
> proxmox_backup::tape::create_drive_state_dir()?;
> diff --git a/src/server/mod.rs b/src/server/mod.rs
> index deeb3398..ffb26f0e 100644
> --- a/src/server/mod.rs
> +++ b/src/server/mod.rs
> @@ -4,7 +4,7 @@
> //! services. We want async IO, so this is built on top of
> //! tokio/hyper.
>
> -use anyhow::Error;
> +use anyhow::{format_err, Error};
> use serde_json::Value;
>
> use proxmox::tools::fs::{create_path, CreateOptions};
> @@ -71,3 +71,17 @@ pub fn create_state_dir() -> Result<(), Error> {
> create_path(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR_M!(), None, Some(opts))?;
> Ok(())
> }
> +
> +/// Create active operations dir with correct permission.
> +pub fn create_active_operations_dir() -> Result<(), Error> {
> + let backup_user = pbs_config::backup_user()?;
> + let mode = nix::sys::stat::Mode::from_bits_truncate(0o0750);
> + let options = CreateOptions::new()
> + .perm(mode)
> + .owner(backup_user.uid)
> + .group(backup_user.gid);
> +
> + create_path(pbs_datastore::ACTIVE_OPERATIONS_DIR, None, Some(options))
> + .map_err(|err: Error| format_err!("unable to create active operations dir - {}", err))?;
> + Ok(())
> +}
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [PATCH v4 proxmox-backup 4/6] api2: make maintenance_type updatable
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (2 preceding siblings ...)
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 3/6] pbs-datastore: add active operations tracking Hannes Laimer
@ 2021-11-12 12:30 ` Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 5/6] api2: add get_active_operations endpoint Hannes Laimer
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2021-11-12 12:30 UTC (permalink / raw)
To: pbs-devel
---
v3->v4: remove check for conflicting tasks. Tasks should not perform
lookups after they have started, therefore alreday running tasks will
not be interupted by an updated maintenance type.
src/api2/config/datastore.rs | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index b9367469..b410a31f 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -183,6 +183,8 @@ pub enum DeletableProperty {
notify_user,
/// Delete the notify property
notify,
+ /// Delete the maintenance-type property
+ maintenance_type,
}
#[api(
@@ -249,6 +251,7 @@ pub fn update_datastore(
DeletableProperty::verify_new => { data.verify_new = None; },
DeletableProperty::notify => { data.notify = None; },
DeletableProperty::notify_user => { data.notify_user = None; },
+ DeletableProperty::maintenance_type => { data.maintenance_type = None; },
}
}
}
@@ -294,6 +297,8 @@ pub fn update_datastore(
if update.notify_user.is_some() { data.notify_user = update.notify_user; }
+ if update.maintenance_type.is_some() { data.maintenance_type = update.maintenance_type; }
+
config.set_data(&name, "datastore", &data)?;
pbs_config::datastore::save_config(&config)?;
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH v4 proxmox-backup 5/6] api2: add get_active_operations endpoint
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (3 preceding siblings ...)
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 4/6] api2: make maintenance_type updatable Hannes Laimer
@ 2021-11-12 12:30 ` Hannes Laimer
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 6/6] ui: add option to change the maintenance type Hannes Laimer
2021-11-22 7:28 ` [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Thomas Lamprecht
6 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2021-11-12 12:30 UTC (permalink / raw)
To: pbs-devel
---
new in v4: needed for displaying the currently conbflicting tasks when
setting a new maintenance type.
pbs-datastore/src/datastore.rs | 41 ++++++++++++++++++++++++++++++++++
pbs-datastore/src/lib.rs | 2 +-
src/api2/admin/datastore.rs | 31 +++++++++++++++++++++++++
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index d8c2f3f5..a0cc1a03 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -79,6 +79,47 @@ impl Clone for DataStore {
}
}
+pub fn get_active_operations(name: &str, operation: Operation) -> Result<i64, Error> {
+ let mut path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
+ let mut lock_path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
+ path.push(name);
+ lock_path.push(format!(".{}.lock", name));
+
+ let user = pbs_config::backup_user()?;
+ let options = proxmox::tools::fs::CreateOptions::new()
+ .group(user.gid)
+ .owner(user.uid)
+ .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
+
+ let timeout = std::time::Duration::new(10, 0);
+ proxmox::tools::fs::open_file_locked(&lock_path, timeout, true, options.clone())?;
+
+ match proxmox::tools::fs::file_get_optional_contents(&path) {
+ Ok(Some(data)) => {
+ let mut count = 0;
+ if let Ok(str_data) = String::from_utf8(data) {
+ for line in str_data.lines() {
+ let split = line.split(" ").collect::<Vec<&str>>();
+ match split[0].parse::<i32>() {
+ Ok(line_pid)
+ if procfs::check_process_running(line_pid).is_some()
+ && split.len() == 3 =>
+ {
+ count += match operation {
+ Operation::Read => split[1].parse::<i64>().unwrap_or(0),
+ Operation::Write => split[2].parse::<i64>().unwrap_or(0),
+ }
+ }
+ _ => {}
+ }
+ }
+ };
+ Ok(count)
+ }
+ _ => Ok(0),
+ }
+}
+
fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> {
let mut path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
let mut lock_path = PathBuf::from(crate::ACTIVE_OPERATIONS_DIR);
diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
index 159642fd..c0f9c0d8 100644
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -200,7 +200,7 @@ pub use manifest::BackupManifest;
pub use store_progress::StoreProgress;
mod datastore;
-pub use datastore::{check_backup_owner, DataStore};
+pub use datastore::{get_active_operations, check_backup_owner, DataStore};
mod snapshot_reader;
pub use snapshot_reader::SnapshotReader;
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 02a42369..ab962e23 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1576,6 +1576,32 @@ pub fn get_rrd_stats(
)
}
+#[api(
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, true),
+ },
+)]
+/// Read datastore stats
+pub fn get_active_operations(
+ store: String,
+ _param: Value,
+) -> Result<Value, Error> {
+
+ let reading = pbs_datastore::get_active_operations(&store, Operation::Read)?;
+ let writing = pbs_datastore::get_active_operations(&store, Operation::Write)?;
+ Ok(json!({
+ "read": reading,
+ "write": writing
+ }))
+}
+
#[api(
input: {
properties: {
@@ -1933,6 +1959,11 @@ pub fn set_backup_owner(
#[sortable]
const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
+ (
+ "active-operations",
+ &Router::new()
+ .get(&API_METHOD_GET_ACTIVE_OPERATIONS)
+ ),
(
"catalog",
&Router::new()
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [pbs-devel] [PATCH v4 proxmox-backup 6/6] ui: add option to change the maintenance type
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (4 preceding siblings ...)
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 5/6] api2: add get_active_operations endpoint Hannes Laimer
@ 2021-11-12 12:30 ` Hannes Laimer
2021-11-22 7:28 ` [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Thomas Lamprecht
6 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2021-11-12 12:30 UTC (permalink / raw)
To: pbs-devel
---
The renderer is set in the init function because a context where view is
accessable was needed. Since only the renderer function needs this
context the rest of the 'maintenance-type' row is defined declaratively.
I also tried to somehow inject the active operations values into the
store, since its passed to the renderer function, but that seemed a little
'hacky'.
www/Makefile | 1 +
www/Utils.js | 23 ++++++++++
www/datastore/OptionView.js | 30 +++++++++++++
www/window/MaintenanceOptions.js | 72 ++++++++++++++++++++++++++++++++
4 files changed, 126 insertions(+)
create mode 100644 www/window/MaintenanceOptions.js
diff --git a/www/Makefile b/www/Makefile
index 4aec6e2c..8f6b17ed 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -63,6 +63,7 @@ JSSRC= \
window/BackupGroupChangeOwner.js \
window/CreateDirectory.js \
window/DataStoreEdit.js \
+ window/MaintenanceOptions.js \
window/NotesEdit.js \
window/RemoteEdit.js \
window/NotifyOptions.js \
diff --git a/www/Utils.js b/www/Utils.js
index 36a94211..db23645a 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -640,4 +640,27 @@ Ext.define('PBS.Utils', {
return `${icon} ${value}`;
},
+ renderMaintenance: function(type, activeTasks) {
+ if (!type) return gettext('None');
+ let at = 0;
+ for (let x of ['read-only-', 'offline-']) {
+ if (type.startsWith(x)) {
+ at = x.length;
+ }
+ }
+
+ const conflictingTasks = activeTasks.write + (type.startsWith('offline') ? activeTasks.read : 0);
+ const checkmarkIcon = '<i class="fa fa-check"></i>';
+ const spinnerIcon = '<i class="fa fa-spinner fa-pulse fa-fw"></i>';
+ const conflictingTasksMessage = `<i>${conflictingTasks} conflicting tasks still active</i>`;
+ const extra = conflictingTasks > 0 ? `| ${spinnerIcon} ${conflictingTasksMessage}` : checkmarkIcon;
+
+ const mode = type.substring(0, at-1);
+ let msg = type.substring(at);
+ if (msg.length > 0) {
+ msg = `"${msg}"`;
+ }
+ return Ext.String.capitalize(`${gettext(mode)} ${msg} ${extra}`) || gettext('None');
+ },
+
});
diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 5a5e85be..50ccce47 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -1,3 +1,4 @@
+
Ext.define('PBS.Datastore.Options', {
extend: 'Proxmox.grid.ObjectGrid',
xtype: 'pbsDatastoreOptionView',
@@ -6,6 +7,10 @@ Ext.define('PBS.Datastore.Options', {
cbindData: function(initial) {
let me = this;
+ me.maintenanceActiveTasks = {
+ read: 0,
+ write: 0,
+ };
me.datastore = encodeURIComponent(me.datastore);
me.url = `/api2/json/config/datastore/${me.datastore}`;
me.editorConfig = {
@@ -18,6 +23,24 @@ Ext.define('PBS.Datastore.Options', {
controller: {
xclass: 'Ext.app.ViewController',
+ init: function(view) {
+ let me = this;
+ me.callParent();
+ view.rows['maintenance-type'].renderer =
+ (value) => PBS.Utils.renderMaintenance(value, view.maintenanceActiveTasks);
+
+ me.activeOperationsRstore = Ext.create('Proxmox.data.ObjectStore', {
+ url: `/api2/json/admin/datastore/${view.datastore}/active-operations`,
+ interval: 3000,
+ });
+ me.activeOperationsRstore.startUpdate();
+
+ view.mon(me.activeOperationsRstore, 'load', (store, data, success) => {
+ me.view.maintenanceActiveTasks.read = data[0].data.value;
+ me.view.maintenanceActiveTasks.write = data[1].data.value;
+ });
+ },
+
edit: function() {
this.getView().run_editor();
},
@@ -111,5 +134,12 @@ Ext.define('PBS.Datastore.Options', {
},
},
},
+ "maintenance-type": {
+ required: true,
+ header: gettext('Maintenance mode'),
+ editor: {
+ xtype: 'pbsMaintenanceOptionEdit',
+ },
+ },
},
});
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
new file mode 100644
index 00000000..374b69f6
--- /dev/null
+++ b/www/window/MaintenanceOptions.js
@@ -0,0 +1,72 @@
+Ext.define('PBS.form.oaintenanceType', {
+ extend: 'Proxmox.form.KVComboBox',
+ alias: 'widget.pbsMaintenanceType',
+
+ comboItems: [
+ ['__default__', gettext('None')],
+ ['read-only', gettext('Read only')],
+ ['offline', gettext('Offline')],
+ ],
+});
+
+Ext.define('PBS.window.MaintenanceOptions', {
+ extend: 'Proxmox.window.Edit',
+ xtype: 'pbsMaintenanceOptionEdit',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ subject: gettext('Maintenance mode'),
+
+ width: 450,
+ fieldDefaults: {
+ labelWidth: 120,
+ },
+
+ items: {
+ onGetValues: function(values) {
+ console.log(values);
+ if (values['maintenance-type']) {
+ values['maintenance-type'] =
+ `${values['maintenance-type']}-${values['maintenance-msg'] || ''}`;
+ }
+ return values;
+ },
+ xtype: 'inputpanel',
+ items: [
+ {
+ xtype: 'pbsMaintenanceType',
+ name: 'maintenance-type',
+ fieldLabel: gettext('Maintenance Type'),
+ value: '__default__',
+ deleteEmpty: true,
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ name: 'maintenance-msg',
+ fieldLabel: gettext('Description'),
+ },
+ ],
+ },
+ setValues: function(values) {
+ let me = this;
+
+ let options = {
+ 'maintenance-type': '__default__',
+ 'maintenance-msg': '',
+ };
+ if (values['maintenance-type']) {
+ let type = values['maintenance-type'];
+ let at = 0;
+ for (let x of ['read-only-', 'offline-']) {
+ if (type.startsWith(x)) {
+ at = x.length;
+ }
+ }
+ options = {
+ 'maintenance-type': type.substr(0, at-1) || '__default__',
+ 'maintenance-msg': type.substr(at) || '',
+ };
+ }
+
+ me.callParent([options]);
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore
2021-11-12 12:30 [pbs-devel] [PATCH v4 proxmox-backup 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (5 preceding siblings ...)
2021-11-12 12:30 ` [pbs-devel] [PATCH v4 proxmox-backup 6/6] ui: add option to change the maintenance type Hannes Laimer
@ 2021-11-22 7:28 ` Thomas Lamprecht
6 siblings, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2021-11-22 7:28 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
On 12.11.21 13:30, Hannes Laimer wrote:
> Adds maintenance mode and tracking of active reading/writing operations.
> The maintenance mode prevents the start of new operations if the type of
> operation they would perform on the datastore would conflict with the
> maintenance type that is currently set. This check is performed when
> lookup_datastore is called. Tasks only call this function once at the
> beginning, therefore updating the maintenance type cannot interfere with
> already running tasks.
>
> active operations tracking:
> Changed file layout to now also keep track of the pid and the counts of
> operations that that pid started, like this it is possible to not count
> operations that were started by a dead process, since they are also not
> active anymore. Whenever the file is updated, also entries of dead
note that PIDs can be reused, normally one also safes the processes start-time
to check for (that's theoretically also racy, but in practice no one can spin
that many processes up to force reuse in such a short time).
FWIW, your use case here would actually just require the processes start time,
you can then simply check if it's older than the one of the current process,
if it is, it'd be out of date.
> processes are removed. When the file is read, only entries of active
> processes are counted.
>
> The UI shows a spinner with the count of conflictintg tasks (the tasks
> that were started before the maintenance type was updated) next to it.
> As soon as all conflicting tasks are finished a checkmark appears.
>
It did not applied cleanly anymore, so I resolved the merge conflicts, fixed
a compile error (operations.clone() cannot work as is) applied Dominik's fixes
and cleanups, and did another quick round of cleanup myself, reducing indentation
depth and line count quite a bit, pushed that out to my internal staff repository
as: hannes/datastore-maintenance-v4-with-cleanups
Anyhow, the main reason I did not went for this now is the manual file format
parser, with serde and now also the shared-memory stuff from Dietmar available
we have way better approaches that keeps us more flexible and normally gets a
more efficient parser plus more friendly working with the data as programmer.
So, please:
* check out my branch (you can squash stuff in, at least mine, if you want)
the most important thing is that you ensure Dominik's patch that fixes
tracking for tape is not lost, as without that it won't work as you was
reminded twice by Dominik, IIRC.
* create an actual struct for the active operations tracking and either
- derive serialize and de-serialize and just save/load it as json
- use the shared memory infrastructure, it should cover your use case
relatively fine and may be slightly cheaper (as we save the parse step
completely)
* see if we can move that tracking related stuff in its own file, datastore.rs
is already relatively big..
As said, most of it was ok-ish as is and could have been followed up, but I just
do not want another self-rolled format, that is hard to expand nicely, out there,
especially not as the rust infrastructure we have just makes it so simply to do
it in a nicer way.
^ permalink raw reply [flat|nested] 9+ messages in thread