From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; Thu, 29 Oct 2020 10:50:14 +0100 (CET)
From: Dominik Csapak <d.csapak@proxmox.com>
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
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=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 <d.csapak@proxmox.com>
---
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<HashMap<usize, Arc<WorkerTask>>> = 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<TaskListInfo> = read_task_file_from_path(PROXMOX_BACKUP_INDEX_TASK_FN)?;
+ let had_index_file = !finish_list.is_empty();
+
let mut active_list: Vec<TaskListInfo> = 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<TaskListInfo>,
- file: TaskFile,
+ end: bool,
archive: Option<LogRotateFiles>,
lock: Option<File>,
}
@@ -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