[PATCH datacenter-manager 0/6] fix #7639: task cache: consider running tasks when updating the cutoff timestamp

[PATCH datacenter-manager 0/6] fix #7639: task cache: consider running tasks when updating the cutoff timestamp

[Lukas Wagner]

At the moment, PDM only polls finished tasks from remote nodes.

Considering two tasks:
A: starts at 100, ends at 300
B: starts ad 150, ends at 200

If PDM polls the node at t = 250, it would see that B has finished and
update the cutoff timestamp for the next poll to 150. However, since A
has a starttime of 100, and the cutoff determines the minimum starttime
of tasks to retrieve from the remote via the API, PDM would never fetch
task A, leading to gaps in the task archive.

The solution is to fetch all tasks, running and finished, and then
update the cutoff with the oldest running active task in mind.

A side effect is that we now also return foreign (as in, not started by
PDM) running tasks from the API, which makes them appear in the UI as
well. This was always intended as a future extension anyways, so this
should be okay.

To avoid polled active tasks appearing as 'stuck' until the next regular poll,
we also lower the interval in which we poll foreign active tasks to 30 seconds.


NOTE to testers/reviewers:

An easy way to observe the previously missing tasks is to use the 'Guest
Console' as a task with manually controllable duration.

For instance:

  - Start console for VM 100 in the PVE Web UI
  - Start console for VM 101 in the PVE Web UI
   -> both start a task that runs as long as the browser actively shows the console
   
  - Close console for VM 101
  - Force-refresh the task view in PDM
   -> the console task for VM 101 should now appear (it is finished)
  - Close console for VM 100
  - Force-refresh the task view in PDM
   -> the console task does NOT show up, since the cutoff has already been set
      to the start time of the task for VM 101, which started *later*


Reported in the community forum:

https://forum.proxmox.com/threads/180317/


proxmox-datacenter-manager:

Lukas Wagner (6):
  pdm-api-types: add NativeUpid::node() convenience getter
  task cache: tests: allow to provide an explicit end time in 'task'
    helper
  task cache: tests: add get_cutoff helper
  task cache: tests: add 'make_cache' convenience helper
  fix #7639: task cache: consider running tasks when updating the cutoff
    timestamp
  task cache: poll known active tasks every 30 seconds

 lib/pdm-api-types/src/remote_upid.rs    |  10 +
 server/src/remote_tasks/refresh_task.rs |  13 +-
 server/src/remote_tasks/task_cache.rs   | 252 ++++++++++++++----------
 3 files changed, 159 insertions(+), 116 deletions(-)


Summary over all repositories:
  3 files changed, 159 insertions(+), 116 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH datacenter-manager 1/6] pdm-api-types: add NativeUpid::node() convenience getter

[Lukas Wagner]

Some callers need to get the 'node' property of a UPID, but handle both,
PBS and PVE equally, making a distinction at the call site inconvenient.
Providing a 'node' getter in NativeUpid simplifies callers a little.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 lib/pdm-api-types/src/remote_upid.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/pdm-api-types/src/remote_upid.rs b/lib/pdm-api-types/src/remote_upid.rs
index 1e90bef4..194e4a18 100644
--- a/lib/pdm-api-types/src/remote_upid.rs
+++ b/lib/pdm-api-types/src/remote_upid.rs
@@ -26,6 +26,16 @@ pub enum NativeUpid {
     PbsUpid(pbs_api_types::UPID),
 }
 
+impl NativeUpid {
+    /// Convenience getter to query the 'node' property of a task.
+    pub fn node(&self) -> &str {
+        match self {
+            NativeUpid::PveUpid(upid) => upid.node.as_str(),
+            NativeUpid::PbsUpid(upid) => upid.node.as_str(),
+        }
+    }
+}
+
 impl RemoteUpid {
     /// Create a new remote UPID.
     pub fn new(remote: String, remote_type: RemoteType, upid: String) -> Self {
-- 
2.47.3

[PATCH datacenter-manager 2/6] task cache: tests: allow to provide an explicit end time in 'task' helper

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/remote_tasks/task_cache.rs | 42 +++++++++++++++------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_tasks/task_cache.rs
index b8122d6a..fb544c6a 100644
--- a/server/src/remote_tasks/task_cache.rs
+++ b/server/src/remote_tasks/task_cache.rs
@@ -1265,11 +1265,11 @@ mod tests {
         Ok(())
     }
 
-    fn task(starttime: i64, ended: bool) -> TaskCacheItem {
-        let (status, endtime) = if ended {
-            (Some("OK".into()), Some(starttime + 10))
+    fn task(starttime: i64, endtime: Option<i64>) -> TaskCacheItem {
+        let status = if endtime.is_some() {
+            Some("OK".into())
         } else {
-            (None, None)
+            None
         };
 
         TaskCacheItem {
@@ -1320,7 +1320,7 @@ mod tests {
         cache.new_file(1000, false)?;
         assert_eq!(cache.cache.archive_files(&cache.lock)?.len(), 1);
 
-        add_tasks(&cache, vec![task(1000, true), task(1001, true)])?;
+        add_tasks(&cache, vec![task(1000, Some(1010)), task(1001, Some(1011))])?;
 
         assert_eq!(
             cache.read_state().cutoff_timestamp("pve-remote", "pve"),
@@ -1331,8 +1331,8 @@ mod tests {
 
         assert_eq!(cache.cache.archive_files(&cache.lock)?.len(), 2);
 
-        add_tasks(&cache, vec![task(1500, true), task(1501, true)])?;
-        add_tasks(&cache, vec![task(1200, true), task(1300, true)])?;
+        add_tasks(&cache, vec![task(1500, Some(1510)), task(1501, Some(1511))])?;
+        add_tasks(&cache, vec![task(1200, Some(1210)), task(1300, Some(1310))])?;
 
         assert_eq!(
             cache.read_state().cutoff_timestamp("pve-remote", "pve"),
@@ -1342,15 +1342,15 @@ mod tests {
         cache.rotate(2000)?;
         assert_eq!(cache.cache.archive_files(&cache.lock)?.len(), 3);
 
-        add_tasks(&cache, vec![task(2000, true)])?;
-        add_tasks(&cache, vec![task(1502, true)])?;
-        add_tasks(&cache, vec![task(1002, true)])?;
+        add_tasks(&cache, vec![task(2000, Some(2010))])?;
+        add_tasks(&cache, vec![task(1502, Some(1512))])?;
+        add_tasks(&cache, vec![task(1002, Some(1012))])?;
 
         // These are before the cut-off of 1000, so they will be discarded.
         // add_tasks(&cache, vec![task(800, true), task(900, true)])?;
 
         // This one should be deduped
-        add_tasks(&cache, vec![task(1000, true)])?;
+        add_tasks(&cache, vec![task(1000, Some(1010))])?;
 
         assert_starttimes(
             &cache,
@@ -1386,10 +1386,10 @@ mod tests {
         .write()?;
 
         cache.new_file(1000, false)?;
-        add_tasks(&cache, vec![task(1000, false), task(1001, false)])?;
+        add_tasks(&cache, vec![task(1000, None), task(1001, None)])?;
         assert_eq!(cache.get_tasks(GetTasks::Active)?.count(), 2);
 
-        add_tasks(&cache, vec![task(1000, true), task(1001, true)])?;
+        add_tasks(&cache, vec![task(1000, Some(1010)), task(1001, Some(1011))])?;
 
         assert_starttimes(&cache, &[1001, 1000]);
 
@@ -1417,7 +1417,11 @@ mod tests {
 
         add_tasks(
             &cache,
-            vec![task(1050, true), task(950, true), task(850, true)],
+            vec![
+                task(1050, Some(1060)),
+                task(950, Some(960)),
+                task(850, Some(860)),
+            ],
         )?;
 
         assert_eq!(cache.get_tasks(GetTasks::Archived)?.count(), 3);
@@ -1426,7 +1430,7 @@ mod tests {
     }
 
     fn add_finished_tracked(cache: &WritableTaskCache, starttime: i64) -> Result<(), Error> {
-        let t = task(starttime, true);
+        let t = task(starttime, Some(starttime + 10));
         let upid = t.upid.clone();
 
         let mut node_map = NodeFetchSuccessMap::default();
@@ -1451,10 +1455,10 @@ mod tests {
 
         cache.init(1000)?;
 
-        cache.add_tracked_task(task(1050, false))?;
+        cache.add_tracked_task(task(1050, Some(1060)))?;
 
         assert_eq!(cache.get_tasks(GetTasks::Active)?.count(), 1);
-        cache.add_tracked_task(task(1060, false))?;
+        cache.add_tracked_task(task(1060, Some(1070)))?;
         assert_eq!(cache.get_tasks(GetTasks::Active)?.count(), 2);
 
         assert_eq!(cache.read_state().tracked_tasks().count(), 2);
@@ -1495,10 +1499,10 @@ mod tests {
         .unwrap()
         .write()?;
 
-        add_tasks(&cache, vec![task(1000, true)])?;
+        add_tasks(&cache, vec![task(1000, Some(1010))])?;
         assert!(cache.journal_size()? > 0);
 
-        add_tasks(&cache, vec![task(1000, true)])?;
+        add_tasks(&cache, vec![task(1000, Some(1010))])?;
 
         assert_eq!(cache.journal_size()?, 0);
 
-- 
2.47.3

[PATCH datacenter-manager 3/6] task cache: tests: add get_cutoff helper

[Lukas Wagner]

This avoids a little bit of code duplication of the hard-coded remote
and node names.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/remote_tasks/task_cache.rs | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_tasks/task_cache.rs
index fb544c6a..7da92e31 100644
--- a/server/src/remote_tasks/task_cache.rs
+++ b/server/src/remote_tasks/task_cache.rs
@@ -1301,6 +1301,13 @@ mod tests {
         cache.update(tasks, &node_map, HashSet::new())
     }
 
+    fn get_cutoff(cache: &WritableTaskCache) -> i64 {
+        cache
+            .read_state()
+            .cutoff_timestamp("pve-remote", "pve")
+            .unwrap_or(0)
+    }
+
     const DEFAULT_MAX_SIZE: u64 = 10000;
 
     #[test]
@@ -1322,10 +1329,7 @@ mod tests {
 
         add_tasks(&cache, vec![task(1000, Some(1010)), task(1001, Some(1011))])?;
 
-        assert_eq!(
-            cache.read_state().cutoff_timestamp("pve-remote", "pve"),
-            Some(1001)
-        );
+        assert_eq!(get_cutoff(&cache), 1001);
 
         cache.rotate(1500)?;
 
@@ -1334,10 +1338,7 @@ mod tests {
         add_tasks(&cache, vec![task(1500, Some(1510)), task(1501, Some(1511))])?;
         add_tasks(&cache, vec![task(1200, Some(1210)), task(1300, Some(1310))])?;
 
-        assert_eq!(
-            cache.read_state().cutoff_timestamp("pve-remote", "pve"),
-            Some(1501),
-        );
+        assert_eq!(get_cutoff(&cache), 1501);
 
         cache.rotate(2000)?;
         assert_eq!(cache.cache.archive_files(&cache.lock)?.len(), 3);
-- 
2.47.3

[PATCH datacenter-manager 4/6] task cache: tests: add 'make_cache' convenience helper

[Lukas Wagner]

Most test use similar settings for instantiating the task cache, so it
makes sense to break it out into a helper.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/remote_tasks/task_cache.rs | 71 +++++++++------------------
 1 file changed, 22 insertions(+), 49 deletions(-)

diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_tasks/task_cache.rs
index 7da92e31..e83a351b 100644
--- a/server/src/remote_tasks/task_cache.rs
+++ b/server/src/remote_tasks/task_cache.rs
@@ -1310,8 +1310,7 @@ mod tests {
 
     const DEFAULT_MAX_SIZE: u64 = 10000;
 
-    #[test]
-    fn test_add_tasks() -> Result<(), Error> {
+    fn make_cache() -> Result<(NamedTempDir, TaskCache), Error> {
         let tmp_dir = NamedTempDir::new()?;
         let cache = TaskCache::new(
             tmp_dir.path(),
@@ -1321,8 +1320,15 @@ mod tests {
             0,
             DEFAULT_MAX_SIZE,
         )
-        .unwrap()
-        .write()?;
+        .unwrap();
+
+        Ok((tmp_dir, cache))
+    }
+
+    #[test]
+    fn test_add_tasks() -> Result<(), Error> {
+        let (_tmp_dir, cache) = make_cache().unwrap();
+        let cache = cache.write().unwrap();
 
         cache.new_file(1000, false)?;
         assert_eq!(cache.cache.archive_files(&cache.lock)?.len(), 1);
@@ -1374,17 +1380,8 @@ mod tests {
 
     #[test]
     fn test_active_tasks_are_migrated_to_archive() -> Result<(), Error> {
-        let tmp_dir = NamedTempDir::new()?;
-        let cache = TaskCache::new(
-            tmp_dir.path(),
-            CreateOptions::new(),
-            3,
-            1,
-            0,
-            DEFAULT_MAX_SIZE,
-        )
-        .unwrap()
-        .write()?;
+        let (_tmp_dir, cache) = make_cache().unwrap();
+        let cache = cache.write().unwrap();
 
         cache.new_file(1000, false)?;
         add_tasks(&cache, vec![task(1000, None), task(1001, None)])?;
@@ -1401,17 +1398,9 @@ mod tests {
 
     #[test]
     fn test_init() -> Result<(), Error> {
-        let tmp_dir = NamedTempDir::new()?;
-        let cache = TaskCache::new(
-            tmp_dir.path(),
-            CreateOptions::new(),
-            3,
-            1,
-            100,
-            DEFAULT_MAX_SIZE,
-        )
-        .unwrap()
-        .write()?;
+        let (_tmp_dir, mut cache) = make_cache().unwrap();
+        cache.rotate_after = 100;
+        let cache = cache.write().unwrap();
 
         cache.init(1000)?;
         assert_eq!(cache.cache.archive_files(&cache.lock)?.len(), 3);
@@ -1442,17 +1431,9 @@ mod tests {
 
     #[test]
     fn test_tracking_tasks() -> Result<(), Error> {
-        let tmp_dir = NamedTempDir::new()?;
-        let cache = TaskCache::new(
-            tmp_dir.path(),
-            CreateOptions::new(),
-            3,
-            1,
-            100,
-            DEFAULT_MAX_SIZE,
-        )
-        .unwrap()
-        .write()?;
+        let (_tmp_dir, mut cache) = make_cache().unwrap();
+        cache.rotate_after = 100;
+        let cache = cache.write().unwrap();
 
         cache.init(1000)?;
 
@@ -1483,22 +1464,14 @@ mod tests {
 
     #[test]
     fn journal_is_applied_if_max_size_exceeded() -> Result<(), Error> {
-        let tmp_dir = NamedTempDir::new()?;
-
         // Should be *just* enough to fit a single task, which means that we apply the journal
         // after adding a second one.
         const ENOUGH_FOR_SINGLE_TASK: u64 = 200;
 
-        let cache = TaskCache::new(
-            tmp_dir.path(),
-            CreateOptions::new(),
-            3,
-            1,
-            100,
-            ENOUGH_FOR_SINGLE_TASK,
-        )
-        .unwrap()
-        .write()?;
+        let (_tmp_dir, mut cache) = make_cache().unwrap();
+        cache.rotate_after = 100;
+        cache.journal_max_size = ENOUGH_FOR_SINGLE_TASK;
+        let cache = cache.write().unwrap();
 
         add_tasks(&cache, vec![task(1000, Some(1010))])?;
         assert!(cache.journal_size()? > 0);
-- 
2.47.3

[PATCH datacenter-manager 5/6] fix #7639: task cache: consider running tasks when updating the cutoff timestamp

[Lukas Wagner]

At the moment, PDM only polls finished tasks from remote nodes.

Considering two tasks:
   A: starts at 100, ends at 300
   B: starts ad 150, ends at 200

If PDM polls the node at t = 250, it would see that B has finished and
update the cutoff timestamp for the next poll to 150. However, since A
has a starttime of 100, and the cutoff determines the minimum starttime
of tasks to retrieve from the remote via the API, PDM would never fetch
task A, leading to gaps in the task archive.

The solution is to fetch all tasks, running and finished, and then
update the cutoff with the oldest running active task in mind.

A side effect is that we now also return foreign (as in, not started by
PDM) running tasks from the API, which makes them appear in the UI as
well. This was always intended as a future extension anyways, so this
should be okay.

Initially reported in in the community forum [0].

[0]: https://forum.proxmox.com/threads/180317/

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/remote_tasks/refresh_task.rs |  11 +--
 server/src/remote_tasks/task_cache.rs   | 122 ++++++++++++++++++------
 2 files changed, 94 insertions(+), 39 deletions(-)

diff --git a/server/src/remote_tasks/refresh_task.rs b/server/src/remote_tasks/refresh_task.rs
index 65572808..c3b005ce 100644
--- a/server/src/remote_tasks/refresh_task.rs
+++ b/server/src/remote_tasks/refresh_task.rs
@@ -284,7 +284,7 @@ async fn fetch_tasks_from_single_node(
     match remote.ty {
         RemoteType::Pve => {
             let params = pve_api_types::ListTasks {
-                source: Some(pve_api_types::ListTasksSource::Archive),
+                source: Some(pve_api_types::ListTasksSource::All),
                 since: Some(since),
                 // If `limit` is not provided, we only receive 50 tasks
                 limit: Some(MAX_TASKS_TO_FETCH),
@@ -315,14 +315,7 @@ async fn fetch_tasks_from_single_node(
                 .get_task_list(params)
                 .await?
                 .into_iter()
-                .filter_map(|task| {
-                    if task.endtime.is_some() {
-                        // We only care about finished tasks.
-                        Some(map_pbs_task(task, remote.id.clone()))
-                    } else {
-                        None
-                    }
-                })
+                .map(|task| map_pbs_task(task, remote.id.clone()))
                 .collect();
 
             Ok(task_list)
diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_tasks/task_cache.rs
index e83a351b..04e3d3ff 100644
--- a/server/src/remote_tasks/task_cache.rs
+++ b/server/src/remote_tasks/task_cache.rs
@@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};
 
 use proxmox_sys::fs::CreateOptions;
 
-use pdm_api_types::{NativeUpid, RemoteUpid};
+use pdm_api_types::RemoteUpid;
 
 /// Filename for the file containing running tasks.
 const ACTIVE_FILENAME: &str = "active";
@@ -102,22 +102,34 @@ impl State {
         self.tracked_tasks.remove(upid);
     }
 
-    /// Update the per-node cutoff timestamp if it is higher than the current one.
-    fn update_cutoff_timestamp(&mut self, remote_id: &str, node: &str, starttime: i64) {
+    /// Update the per-node cutoff timestamp with a provided update function.
+    fn update_cutoff_timestamp<F: Fn(Option<i64>) -> i64>(
+        &mut self,
+        remote_id: &str,
+        node: &str,
+        update_fn: F,
+    ) {
         match self.remote_state.get_mut(remote_id) {
             Some(remote_state) => match remote_state.node_state.get_mut(node) {
                 Some(node_state) => {
-                    node_state.cutoff = node_state.cutoff.max(starttime);
+                    node_state.cutoff = update_fn(Some(node_state.cutoff));
                 }
                 None => {
-                    remote_state
-                        .node_state
-                        .insert(node.to_string(), NodeState { cutoff: starttime });
+                    remote_state.node_state.insert(
+                        node.to_string(),
+                        NodeState {
+                            cutoff: update_fn(None),
+                        },
+                    );
                 }
             },
             None => {
-                let node_state =
-                    HashMap::from_iter([(node.to_string(), NodeState { cutoff: starttime })]);
+                let node_state = HashMap::from_iter([(
+                    node.to_string(),
+                    NodeState {
+                        cutoff: update_fn(None),
+                    },
+                )]);
 
                 self.remote_state
                     .insert(remote_id.to_string(), RemoteState { node_state });
@@ -389,7 +401,7 @@ impl WritableTaskCache {
 
     fn write_tasks_to_journal(
         &self,
-        tasks: Vec<TaskCacheItem>,
+        finished_tasks: Vec<TaskCacheItem>,
         active_tasks: &mut HashMap<RemoteUpid, TaskCacheItem>,
         node_success_map: &NodeFetchSuccessMap,
         state: &mut State,
@@ -400,37 +412,53 @@ impl WritableTaskCache {
             .create(true)
             .open(filename)?;
 
-        for task in tasks {
+        for task in finished_tasks {
             // Remove this finished task from our set of active tasks.
             active_tasks.remove(&task.upid);
 
-            match task.upid.native_upid() {
-                Ok(NativeUpid::PveUpid(upid)) => {
-                    let node = &upid.node;
-                    let remote = task.upid.remote();
-
-                    if node_success_map.node_successful(remote, node) {
-                        state.update_cutoff_timestamp(remote, node, task.starttime);
-                    }
-                }
-                Ok(NativeUpid::PbsUpid(upid)) => {
-                    let node = &upid.node;
-                    let remote = task.upid.remote();
-
-                    if node_success_map.node_successful(remote, node) {
-                        state.update_cutoff_timestamp(remote, node, task.starttime);
-                    }
-                }
-                Err(error) => {
-                    log::error!("could not parse PVE UPID - not saving to task cache: {error:#}");
+            let native_upid = match task.upid.native_upid() {
+                Ok(native_upid) => native_upid,
+                Err(err) => {
+                    log::error!("could not deserialize UPID: {err:#}");
                     continue;
                 }
+            };
+
+            let node = native_upid.node();
+            let remote = task.upid.remote();
+
+            if node_success_map.node_successful(remote, node) {
+                state.update_cutoff_timestamp(remote, node, |existing| {
+                    existing.unwrap_or(0).max(task.starttime)
+                });
             }
 
             serde_json::to_writer(&mut file, &task)?;
             writeln!(&file)?;
         }
 
+        // For all remaining active tasks, set the cutoff timestamp to the
+        // start time of the *oldest* running task. This is to avoid
+        // gaps in the task archive if tasks run overlappingly
+        for task in active_tasks.values() {
+            let native_upid = match task.upid.native_upid() {
+                Ok(native_upid) => native_upid,
+                Err(err) => {
+                    log::error!("could not deserialize UPID: {err:#}");
+                    continue;
+                }
+            };
+
+            let node = native_upid.node();
+            let remote = task.upid.remote();
+
+            if node_success_map.node_successful(remote, node) {
+                state.update_cutoff_timestamp(remote, node, |existing| {
+                    existing.unwrap_or(i64::MAX).min(task.starttime)
+                });
+            }
+        }
+
         file.sync_all()?;
 
         Ok(())
@@ -1482,4 +1510,38 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn cutoff_is_oldest_running_task() {
+        let (_tmp_dir, cache) = make_cache().unwrap();
+        let cache = cache.write().unwrap();
+
+        cache.init(500).unwrap();
+
+        // Establish the baseline cutoff with a single, finished task
+        add_tasks(&cache, vec![task(900, Some(910))]).unwrap();
+        assert_eq!(get_cutoff(&cache), 900);
+
+        // Add two running tasks
+        add_tasks(&cache, vec![task(1000, None), task(1100, None)]).unwrap();
+        assert_eq!(cache.get_tasks(GetTasks::Active).unwrap().count(), 2);
+        assert_eq!(cache.get_tasks(GetTasks::Archived).unwrap().count(), 1);
+
+        // Two new *running* tasks, cutoff should remain the same
+        assert_eq!(get_cutoff(&cache), 900);
+
+        add_tasks(&cache, vec![task(1000, None), task(1100, Some(1150))]).unwrap();
+        assert_eq!(cache.get_tasks(GetTasks::Active).unwrap().count(), 1);
+        assert_eq!(cache.get_tasks(GetTasks::Archived).unwrap().count(), 2);
+
+        // The cutoff should stick to the *oldest* running task
+        assert_eq!(get_cutoff(&cache), 1000);
+
+        add_tasks(&cache, vec![task(1000, Some(1200)), task(1100, Some(1150))]).unwrap();
+        // If there is no running task anymore, the youngest finished tasks' starttime determines
+        // the cutoff
+        assert_eq!(get_cutoff(&cache), 1100);
+        assert_eq!(cache.get_tasks(GetTasks::Active).unwrap().count(), 0);
+        assert_eq!(cache.get_tasks(GetTasks::Archived).unwrap().count(), 3);
+    }
 }
-- 
2.47.3

[PATCH datacenter-manager 6/6] task cache: poll known active tasks every 30 seconds

[Lukas Wagner]

Since we now have to fetch active tasks anyway to avoid gaps in the
task archive, we might as well lower the polling interval for any known
active tasks to a smaller interval of 30 seconds. This should be a nice
compromise between avoid too many requests and realtime-ness.

The general polling interval for active and archived tasks is still set
to 10 minutes, meaning any task that starts and finishes between polls
is not actively polled. These would simply appear as 'finished' when the
next 10 minute poll is done. Only tasks that are active when this
regular poll happens are affected; the lower polling interval mainly
ensures that any active task does not appear stuck until the next
regular poll happens.

Tasks that were started by PDM, so called 'tracked tasks', continue to
be polled every 10 seconds. These have arguably stricter requirements to
perceived responsiveness.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/remote_tasks/refresh_task.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/src/remote_tasks/refresh_task.rs b/server/src/remote_tasks/refresh_task.rs
index c3b005ce..632b7d52 100644
--- a/server/src/remote_tasks/refresh_task.rs
+++ b/server/src/remote_tasks/refresh_task.rs
@@ -28,7 +28,7 @@ const TASK_FETCH_INTERVAL: Duration = Duration::from_secs(600);
 // NOTE: Since we at the moment never query active tasks from remotes, this is merely a safeguard
 // to clear stuck active tasks from a previous bug. If we at some point query active tasks, we
 // might lower this interval.
-const POLL_ACTIVE_INTERVAL: Duration = Duration::from_secs(600);
+const POLL_ACTIVE_INTERVAL: Duration = Duration::from_secs(30);
 
 /// Interval at which to check for task cache rotation.
 const CHECK_ROTATE_INTERVAL: Duration = Duration::from_secs(3600);
-- 
2.47.3