[pbs-devel] [PATCH proxmox-backup 1/3] docs: fix #2851 Add note about GC grace period

[pbs-devel] [PATCH proxmox-backup 1/3] docs: fix #2851 Add note about GC grace period

[Aaron Lauterer]

Adding a note about the garbage collection's grace period due to the
default atime behavior should help to avoid confusion as to why space is
not freed immediately.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

I hope I got the technical details right.

I did put the section in a note for now but I am not sure if it is the
best way to present that information.

 docs/administration-guide.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/administration-guide.rst b/docs/administration-guide.rst
index 254d36e1..a0ac8c79 100644
--- a/docs/administration-guide.rst
+++ b/docs/administration-guide.rst
@@ -962,6 +962,17 @@ unused data blocks are removed.
   depending on the number of chunks and the speed of the underlying
   disks.
 
+.. note:: The garbage collection will only remove chunks that haven't been used
+   for at least one day (exactly 24h 5m). This grace period is necessary because
+   chunks in use are marked by touching the chunk which updates the ``atime``
+   (access time) property. Filesystems are mounted with the ``relatime`` option
+   by default. This results in a better performance by only updating the
+   ``atime`` property if the last access has been at least 24 hours ago. The
+   downside is, that touching a chunk within these 24 hours will not update its
+   ``atime`` property.
+
+   If there are chunks in the grace period, it will be logged at the end of the
+   garbage collection run as *Pending removals*.
 
 .. code-block:: console
 
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/3] backup: Fix typos and grammar

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/backup.rs | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/backup.rs b/src/backup.rs
index 10a65d6c..cb1426ab 100644
--- a/src/backup.rs
+++ b/src/backup.rs
@@ -40,21 +40,21 @@
 //!
 //!   Acquire shared lock for ChunkStore (process wide).
 //!
-//!   Note: When creating .idx files, we create temporary (.tmp) file,
+//!   Note: When creating .idx files, we create temporary a (.tmp) file,
 //!   then do an atomic rename ...
 //!
 //!
 //! * Garbage Collect:
 //!
 //!   Acquire exclusive lock for ChunkStore (process wide). If we have
-//!   already an shared lock for ChunkStore, try to updraged that
+//!   already a shared lock for the ChunkStore, try to upgrade that
 //!   lock.
 //!
 //!
 //! * Server Restart
 //!
-//!   Try to abort running garbage collection to release exclusive
-//!   ChunkStore lock asap. Start new service with existing listening
+//!   Try to abort the running garbage collection to release exclusive
+//!   ChunkStore locks ASAP. Start the new service with the existing listening
 //!   socket.
 //!
 //!
@@ -62,10 +62,10 @@
 //!
 //! Deleting backups is as easy as deleting the corresponding .idx
 //! files. Unfortunately, this does not free up any storage, because
-//! those files just contains references to chunks.
+//! those files just contain references to chunks.
 //!
 //! To free up some storage, we run a garbage collection process at
-//! regular intervals. The collector uses an mark and sweep
+//! regular intervals. The collector uses a mark and sweep
 //! approach. In the first phase, it scans all .idx files to mark used
 //! chunks. The second phase then removes all unmarked chunks from the
 //! store.
@@ -90,12 +90,12 @@
 //! amount of time ago (by default 24h). So we may only delete chunks
 //! with `atime` older than 24 hours.
 //!
-//! Another problem arise from running backups. The mark phase does
+//! Another problem arises from running backups. The mark phase does
 //! not find any chunks from those backups, because there is no .idx
 //! file for them (created after the backup). Chunks created or
 //! touched by those backups may have an `atime` as old as the start
-//! time of those backup. Please not that the backup start time may
-//! predate the GC start time. Se we may only delete chunk older than
+//! time of those backups. Please note that the backup start time may
+//! predate the GC start time. So we may only delete chunks older than
 //! the start time of those running backup jobs.
 //!
 //!
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 3/3] chunk_store: Fix typo in bail message

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/backup/chunk_store.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backup/chunk_store.rs b/src/backup/chunk_store.rs
index 87000630..d57befdd 100644
--- a/src/backup/chunk_store.rs
+++ b/src/backup/chunk_store.rs
@@ -178,7 +178,7 @@ impl ChunkStore {
                 return Ok(false);
             }
 
-            bail!("updata atime failed for chunk {:?} - {}", chunk_path, err);
+            bail!("update atime failed for chunk {:?} - {}", chunk_path, err);
         }
 
         Ok(true)
-- 
2.20.1

[pbs-devel] applied-series: [PATCH proxmox-backup 1/3] docs: fix #2851 Add note about GC grace period

[Thomas Lamprecht]

On 21.07.20 11:54, Aaron Lauterer wrote:
> Adding a note about the garbage collection's grace period due to the
> default atime behavior should help to avoid confusion as to why space is
> not freed immediately.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> 
> I hope I got the technical details right.
> 
> I did put the section in a note for now but I am not sure if it is the
> best way to present that information.
> 
>  docs/administration-guide.rst | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
>

applied series, thanks!