Implementation of backup tree deletion

gustabowill commented 3 months ago

This PR got bigger than I expected so I will try to explain the changes per commit.

In 129c6723302daea0f7a59954fd303b53f578b36c is the implementation for deleting descendant backup nodes when a parent is deleted. To achieve this, I divided the current delete method of the Server into two methods:

delete which will now have some validations to make sure the backup is deletable (this used to be done later in the backup manager) as well as pass the whole chain for deletion inside a server level lock;
perform_delete will be doing some of the same work that the previous delete method was doing i.e. validating file permissions, holding the backup level lock and calling the BackupManager.delete. One thing to notice here is that this code used to acquire a server lock in the beginning, but it was released before the deletion even started , I also think the error message there was not entirely correct as it would say something like "there's another process for this backup id" when the lock was actually on the server, not on the backup. I "fixed" those things, but would like a check-up just to make sure.

In b0f38bef5e6e9dc89fb0c8900b35f7bd796dacc0 I fixed the tests that were broken due to moving validations and locks around. I had to create new tests in the server logic as well as change some existing ones and remove some code from the backup manager tests.

In b41d409bbc62c00509e317be0ac3d1f414da3811 I created new tests for the children deletion logic. It basically tests that the whole chain is being passed for deletion and also that a child deletion also deletes its reference in its parent.

References: BAR-181

barthisrael commented 3 months ago

This seems to be working fine:

My backups (you can guess the relationship based on their names 😆 ):

$ barman list-backups pg17
pg17 20240628T000532 'yet_another_full' - Thu Jun 27 22:05:34 2024 - Size: 251.7 MiB - WAL Size: 0 B - WAITING_FOR_WALS
pg17 20240628T000524 'another_incremental' - Thu Jun 27 22:05:26 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000511 'another_full' - Thu Jun 27 22:05:13 2024 - Size: 251.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000443 'child_2_1' - Thu Jun 27 22:04:44 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000436 'child_1_2' - Thu Jun 27 22:04:37 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000433 'child_1_1' - Thu Jun 27 22:04:34 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000419 'child_2' - Thu Jun 27 22:04:20 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000416 'child_1' - Thu Jun 27 22:04:17 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000321 'full_1' - Thu Jun 27 22:03:25 2024 - Size: 251.7 MiB - WAL Size: 96.0 MiB

Backup deletion starting from a node in the middle of the tree:

$ barman delete pg17 child_2
WARNING: Backup 20240628T000419 has incremental backups which depend on it. Deleting all backups in the tree
Deleting backup 20240628T000443 for server pg17
Deleted backup 20240628T000443 (start time: Fri Jun 28 00:08:05 2024, elapsed time: less than one second)
Deleting backup 20240628T000419 for server pg17
Deleted backup 20240628T000419 (start time: Fri Jun 28 00:08:05 2024, elapsed time: less than one second)

$ barman list-backups pg17
pg17 20240628T000532 'yet_another_full' - Thu Jun 27 22:05:34 2024 - Size: 251.7 MiB - WAL Size: 0 B - WAITING_FOR_WALS
pg17 20240628T000524 'another_incremental' - Thu Jun 27 22:05:26 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000511 'another_full' - Thu Jun 27 22:05:13 2024 - Size: 251.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000436 'child_1_2' - Thu Jun 27 22:04:37 2024 - Size: 35.7 MiB - WAL Size: 64.0 MiB
pg17 20240628T000433 'child_1_1' - Thu Jun 27 22:04:34 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000416 'child_1' - Thu Jun 27 22:04:17 2024 - Size: 35.7 MiB - WAL Size: 64.0 MiB
pg17 20240628T000321 'full_1' - Thu Jun 27 22:03:25 2024 - Size: 251.7 MiB - WAL Size: 96.0 MiB

Backup deletion starting from a leaf in the tree:

$ barman delete pg17 child_1_2
Deleting backup 20240628T000436 for server pg17
Deleted backup 20240628T000436 (start time: Fri Jun 28 00:08:58 2024, elapsed time: less than one second)

$ barman list-backups pg17
pg17 20240628T000532 'yet_another_full' - Thu Jun 27 22:05:34 2024 - Size: 251.7 MiB - WAL Size: 0 B - WAITING_FOR_WALS
pg17 20240628T000524 'another_incremental' - Thu Jun 27 22:05:26 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000511 'another_full' - Thu Jun 27 22:05:13 2024 - Size: 251.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000433 'child_1_1' - Thu Jun 27 22:04:34 2024 - Size: 35.7 MiB - WAL Size: 96.0 MiB
pg17 20240628T000416 'child_1' - Thu Jun 27 22:04:17 2024 - Size: 35.7 MiB - WAL Size: 64.0 MiB
pg17 20240628T000321 'full_1' - Thu Jun 27 22:03:25 2024 - Size: 251.7 MiB - WAL Size: 96.0 MiB

Backup deletion starting from the root:

$ barman delete pg17 full_1
WARNING: Backup 20240628T000321 has incremental backups which depend on it. Deleting all backups in the tree
Deleting backup 20240628T000433 for server pg17
Deleted backup 20240628T000433 (start time: Fri Jun 28 00:09:34 2024, elapsed time: less than one second)
Deleting backup 20240628T000416 for server pg17
Deleted backup 20240628T000416 (start time: Fri Jun 28 00:09:34 2024, elapsed time: less than one second)
Deleting backup 20240628T000321 for server pg17
Delete associated WAL segments:
    000000010000000300000039
    00000001000000030000003A
    00000001000000030000003B
    00000001000000030000003C
    00000001000000030000003D
    00000001000000030000003E
    00000001000000030000003F
    000000010000000300000040
    000000010000000300000041
    000000010000000300000042
    000000010000000300000043
    000000010000000300000044
    000000010000000300000045
    000000010000000300000046
    000000010000000300000047
    000000010000000300000048
Deleted backup 20240628T000321 (start time: Fri Jun 28 00:09:34 2024, elapsed time: less than one second)

$ barman list-backups pg17
pg17 20240628T000532 'yet_another_full' - Thu Jun 27 22:05:34 2024 - Size: 251.7 MiB - WAL Size: 0 B - WAITING_FOR_WALS
pg17 20240628T000524 'another_incremental' - Thu Jun 27 22:05:26 2024 - Size: 35.7 MiB - WAL Size: 32.0 MiB
pg17 20240628T000511 'another_full' - Thu Jun 27 22:05:13 2024 - Size: 251.7 MiB - WAL Size: 32.0 MiB

gustabowill commented 3 months ago

@barthisrael Thanks, I completely forgot about the references 😆. Just pushed the changes you asked for.

barthisrael commented 3 months ago

A random thought that just came up: maybe we need to handle this also in Barman cloud?

gcalacoci commented 3 months ago

A random thought that just came up: maybe we need to handle this also in Barman cloud?

IMHO barman cloud and incremental is something that is going to come later. we first focus on local, and then add cloud. IIRC we discussed this with @martinmarques during the design.

barthisrael commented 3 months ago

IMHO barman cloud and incremental is something that is going to come later. we first focus on local, and then add cloud. IIRC we discussed this with @martinmarques during the design.

I really don't recall this specific detail, sorry. IAC what you said makes sense to me, as we discussed about implementing this in incremental steps.

EnterpriseDB / barman

Implementation of backup tree deletion #946