[pbs-devel] [PATCH proxmox-backup] task tracking: fix adding new entry if other PID is tracked

[pbs-devel] [PATCH proxmox-backup] task tracking: fix adding new entry if other PID is tracked

[Fabian Grünbichler]

if the tracking file contains an entry for another, still running PID, that
entry must be preserved, but a new entry for the current PID should still be
inserted..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
found while benchmarking Samuel's datastore lookup caching series..

 pbs-datastore/src/task_tracking.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pbs-datastore/src/task_tracking.rs b/pbs-datastore/src/task_tracking.rs
index 77851cab6..44a4522dc 100644
--- a/pbs-datastore/src/task_tracking.rs
+++ b/pbs-datastore/src/task_tracking.rs
@@ -108,6 +108,7 @@ pub fn update_active_operations(
         Operation::Write => ActiveOperationStats { read: 0, write: 1 },
         Operation::Lookup => ActiveOperationStats { read: 0, write: 0 },
     };
+    let mut found_entry = false;
     let mut updated_tasks: Vec<TaskOperations> = match file_read_optional_string(&path)? {
         Some(data) => serde_json::from_str::<Vec<TaskOperations>>(&data)?
             .iter_mut()
@@ -116,6 +117,7 @@ pub fn update_active_operations(
                     Some(stat) if pid == task.pid && stat.starttime != task.starttime => None,
                     Some(_) => {
                         if pid == task.pid {
+                            found_entry = true;
                             match operation {
                                 Operation::Read => task.active_operations.read += count,
                                 Operation::Write => task.active_operations.write += count,
@@ -132,7 +134,7 @@ pub fn update_active_operations(
         None => Vec::new(),
     };
 
-    if updated_tasks.is_empty() {
+    if !found_entry {
         updated_tasks.push(TaskOperations {
             pid,
             starttime,
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] task tracking: fix adding new entry if other PID is tracked

[Hannes Laimer]

Good catch! Changes LGTM, consider this

Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>

On 11/12/25 14:15, Fabian Grünbichler wrote:
> if the tracking file contains an entry for another, still running PID, that
> entry must be preserved, but a new entry for the current PID should still be
> inserted..
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> found while benchmarking Samuel's datastore lookup caching series..
> 
>   pbs-datastore/src/task_tracking.rs | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-datastore/src/task_tracking.rs b/pbs-datastore/src/task_tracking.rs
> index 77851cab6..44a4522dc 100644
> --- a/pbs-datastore/src/task_tracking.rs
> +++ b/pbs-datastore/src/task_tracking.rs
> @@ -108,6 +108,7 @@ pub fn update_active_operations(
>           Operation::Write => ActiveOperationStats { read: 0, write: 1 },
>           Operation::Lookup => ActiveOperationStats { read: 0, write: 0 },
>       };
> +    let mut found_entry = false;
>       let mut updated_tasks: Vec<TaskOperations> = match file_read_optional_string(&path)? {
>           Some(data) => serde_json::from_str::<Vec<TaskOperations>>(&data)?
>               .iter_mut()
> @@ -116,6 +117,7 @@ pub fn update_active_operations(
>                       Some(stat) if pid == task.pid && stat.starttime != task.starttime => None,
>                       Some(_) => {
>                           if pid == task.pid {
> +                            found_entry = true;
>                               match operation {
>                                   Operation::Read => task.active_operations.read += count,
>                                   Operation::Write => task.active_operations.write += count,
> @@ -132,7 +134,7 @@ pub fn update_active_operations(
>           None => Vec::new(),
>       };
>   
> -    if updated_tasks.is_empty() {
> +    if !found_entry {
>           updated_tasks.push(TaskOperations {
>               pid,
>               starttime,



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] task tracking: fix adding new entry if other PID is tracked

[Samuel Rufinatscha]

Looks good to me!

Reviewed-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>

On 11/12/25 2:15 PM, Fabian Grünbichler wrote:
> if the tracking file contains an entry for another, still running PID, that
> entry must be preserved, but a new entry for the current PID should still be
> inserted..
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> found while benchmarking Samuel's datastore lookup caching series..
> 
>   pbs-datastore/src/task_tracking.rs | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-datastore/src/task_tracking.rs b/pbs-datastore/src/task_tracking.rs
> index 77851cab6..44a4522dc 100644
> --- a/pbs-datastore/src/task_tracking.rs
> +++ b/pbs-datastore/src/task_tracking.rs
> @@ -108,6 +108,7 @@ pub fn update_active_operations(
>           Operation::Write => ActiveOperationStats { read: 0, write: 1 },
>           Operation::Lookup => ActiveOperationStats { read: 0, write: 0 },
>       };
> +    let mut found_entry = false;
>       let mut updated_tasks: Vec<TaskOperations> = match file_read_optional_string(&path)? {
>           Some(data) => serde_json::from_str::<Vec<TaskOperations>>(&data)?
>               .iter_mut()
> @@ -116,6 +117,7 @@ pub fn update_active_operations(
>                       Some(stat) if pid == task.pid && stat.starttime != task.starttime => None,
>                       Some(_) => {
>                           if pid == task.pid {
> +                            found_entry = true;
>                               match operation {
>                                   Operation::Read => task.active_operations.read += count,
>                                   Operation::Write => task.active_operations.write += count,
> @@ -132,7 +134,7 @@ pub fn update_active_operations(
>           None => Vec::new(),
>       };
>   
> -    if updated_tasks.is_empty() {
> +    if !found_entry {
>           updated_tasks.push(TaskOperations {
>               pid,
>               starttime,



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH proxmox-backup] task tracking: fix adding new entry if other PID is tracked

[Fabian Grünbichler]

On Wed, 12 Nov 2025 14:14:43 +0100, Fabian Grünbichler wrote:
> if the tracking file contains an entry for another, still running PID, that
> entry must be preserved, but a new entry for the current PID should still be
> inserted..
> 
> 

Applied, thanks for the reviews!

[1/1] task tracking: fix adding new entry if other PID is tracked
      commit: 2ba1090b76d7105af69fca43bfd446a589087163

Best regards,
-- 
Fabian Grünbichler <f.gruenbichler@proxmox.com>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel