From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 9432F63F95 for ; Thu, 29 Oct 2020 10:50:45 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 82E63272FF for ; Thu, 29 Oct 2020 10:50:15 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id D5A56272F7 for ; Thu, 29 Oct 2020 10:50:14 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 98E9345F7E for ; Thu, 29 Oct 2020 10:50:14 +0100 (CET) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Thu, 29 Oct 2020 10:50:13 +0100 Message-Id: <20201029095013.4885-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.446 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup v2] server/worker_task: simplify task log writing X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Oct 2020 09:50:45 -0000 instead of prerotating 1000 tasks (which resulted in 2 writes each time an active worker was finished) simply append finished tasks to the archive (which will be rotated) page cache should be good enough so that we can get the task logs fast since existing installations might have an 'index' file, we still have to read tasks from there, but only if it exists this simplifies the TaskListInfoIterator a good amount Signed-off-by: Dominik Csapak --- we now write all finished tasks into the archive, as soon as any code tries to iterate over any finished task this way we can remove most of the index handling code, besides that tiny bit of compatibility that we can drop with 1.x src/server/worker_task.rs | 91 +++++++++++++-------------------------- 1 file changed, 29 insertions(+), 62 deletions(-) diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs index 8ef0fde7..021751ea 100644 --- a/src/server/worker_task.rs +++ b/src/server/worker_task.rs @@ -35,8 +35,6 @@ pub const PROXMOX_BACKUP_ACTIVE_TASK_FN: &str = concat!(PROXMOX_BACKUP_TASK_DIR_ pub const PROXMOX_BACKUP_INDEX_TASK_FN: &str = concat!(PROXMOX_BACKUP_TASK_DIR_M!(), "/index"); pub const PROXMOX_BACKUP_ARCHIVE_TASK_FN: &str = concat!(PROXMOX_BACKUP_TASK_DIR_M!(), "/archive"); -const MAX_INDEX_TASKS: usize = 1000; - lazy_static! { static ref WORKER_TASK_LIST: Mutex>> = Mutex::new(HashMap::new()); @@ -363,7 +361,10 @@ fn update_active_workers(new_upid: Option<&UPID>) -> Result<(), Error> { let lock = lock_task_list_files(true)?; + // TODO remove with 1.x let mut finish_list: Vec = read_task_file_from_path(PROXMOX_BACKUP_INDEX_TASK_FN)?; + let had_index_file = !finish_list.is_empty(); + let mut active_list: Vec = read_task_file_from_path(PROXMOX_BACKUP_ACTIVE_TASK_FN)? .into_iter() .filter_map(|info| { @@ -412,33 +413,10 @@ fn update_active_workers(new_upid: Option<&UPID>) -> Result<(), Error> { } }); - - let start = if finish_list.len() > MAX_INDEX_TASKS { - finish_list.len() - MAX_INDEX_TASKS - } else { - 0 - }; - - let end = (start+MAX_INDEX_TASKS).min(finish_list.len()); - - let index_raw = if end > start { - render_task_list(&finish_list[start..end]) - } else { - "".to_string() - }; - - replace_file( - PROXMOX_BACKUP_INDEX_TASK_FN, - index_raw.as_bytes(), - CreateOptions::new() - .owner(backup_user.uid) - .group(backup_user.gid), - )?; - - if !finish_list.is_empty() && start > 0 { + if !finish_list.is_empty() { match std::fs::OpenOptions::new().append(true).create(true).open(PROXMOX_BACKUP_ARCHIVE_TASK_FN) { Ok(mut writer) => { - for info in &finish_list[0..start] { + for info in &finish_list { writer.write_all(render_task_line(&info).as_bytes())?; } }, @@ -448,6 +426,12 @@ fn update_active_workers(new_upid: Option<&UPID>) -> Result<(), Error> { nix::unistd::chown(PROXMOX_BACKUP_ARCHIVE_TASK_FN, Some(backup_user.uid), Some(backup_user.gid))?; } + // TODO Remove with 1.x + // for compatibility, if we had an INDEX file, we do not need it anymore + if had_index_file { + let _ = nix::unistd::unlink(PROXMOX_BACKUP_INDEX_TASK_FN); + } + drop(lock); Ok(()) @@ -511,16 +495,9 @@ where read_task_file(file) } -enum TaskFile { - Active, - Index, - Archive, - End, -} - pub struct TaskListInfoIterator { list: VecDeque, - file: TaskFile, + end: bool, archive: Option, lock: Option, } @@ -535,7 +512,10 @@ impl TaskListInfoIterator { .iter() .any(|info| info.state.is_some() || !worker_is_active_local(&info.upid)); - if needs_update { + // TODO remove with 1.x + let index_exists = std::path::Path::new(PROXMOX_BACKUP_INDEX_TASK_FN).is_file(); + + if needs_update || index_exists { drop(lock); update_active_workers(None)?; let lock = lock_task_list_files(false)?; @@ -554,12 +534,11 @@ impl TaskListInfoIterator { Some(logrotate.files()) }; - let file = if active_only { TaskFile::End } else { TaskFile::Active }; let lock = if active_only { None } else { Some(read_lock) }; Ok(Self { list: active_list.into(), - file, + end: active_only, archive, lock, }) @@ -573,35 +552,23 @@ impl Iterator for TaskListInfoIterator { loop { if let Some(element) = self.list.pop_back() { return Some(Ok(element)); + } else if self.end { + return None; } else { - match self.file { - TaskFile::Active => { - let index = match read_task_file_from_path(PROXMOX_BACKUP_INDEX_TASK_FN) { - Ok(index) => index, + if let Some(mut archive) = self.archive.take() { + if let Some(file) = archive.next() { + let list = match read_task_file(file) { + Ok(list) => list, Err(err) => return Some(Err(err)), }; - self.list.append(&mut index.into()); - self.file = TaskFile::Index; - }, - TaskFile::Index | TaskFile::Archive => { - if let Some(mut archive) = self.archive.take() { - if let Some(file) = archive.next() { - let list = match read_task_file(file) { - Ok(list) => list, - Err(err) => return Some(Err(err)), - }; - self.list.append(&mut list.into()); - self.archive = Some(archive); - self.file = TaskFile::Archive; - continue; - } - } - self.file = TaskFile::End; - self.lock.take(); - return None; + self.list.append(&mut list.into()); + self.archive = Some(archive); + continue; } - TaskFile::End => return None, } + + self.end = true; + self.lock.take(); } } } -- 2.20.1