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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 295BE62C0C for ; Wed, 28 Oct 2020 10:58:04 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1DC7E1D896 for ; Wed, 28 Oct 2020 10:58:04 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 83B8E1D880 for ; Wed, 28 Oct 2020 10:58:02 +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 5196A45C7D for ; Wed, 28 Oct 2020 10:58:02 +0100 (CET) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Wed, 28 Oct 2020 10:58:01 +0100 Message-Id: <20201028095801.1737-2-d.csapak@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201028095801.1737-1-d.csapak@proxmox.com> References: <20201028095801.1737-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.449 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] [RFC PATCH proxmox-backup 2/2] 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: Wed, 28 Oct 2020 09:58:04 -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 Signed-off-by: Dominik Csapak --- maybe there is a better way to get rid of the index file handling? we cannot really simply append the index file to the archive in a postinst since the old daemon may still write into it.. do we actually care? the users may lose some task information, but no actual harm should come from lost task logs, since we use the jobstate for the state handling and scheduling we could also simply removing the index code from the iterator and leave it in 'update_active_workers' so that after the next finished active task, we move them from the index file to archive so we lose them just temporarily....? not sure so sending as RFC src/server/worker_task.rs | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs index 8ef0fde7..2e873904 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 2.0? 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 2.0? + // 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(()) @@ -518,6 +502,7 @@ enum TaskFile { End, } +// TODO remove Index with 2.0? pub struct TaskListInfoIterator { list: VecDeque, file: TaskFile, -- 2.20.1