feat(docs): Don't do duplicate snapshot names (#1)

README.md

@@ -105,6 +105,60 @@ Whatever you set as your `pkgs_list_max_length` is still just a best effort as i
 
 Have a look at `pacman-zfs-snapshot.conf` as well, its comments should be clear enough to get you going.
 
+# Avoiding naming collisions
+
+By default snapshot names contain a timestamp formatted like so: `2023-03-07-0114`. This makes snapshot names reasonably unique. You can change both the timestamp format and timezone in `pacman-zfs-snapshot.conf` where the format defaults to:
+```
+snap_date_format='%F-%H%M'
+```
+And the timezone defaults to:
+```
+snap_timezone='Etc/UTC'
+```
+
+With these settings it is possible to cause ZFS snapshot name collisions (meaning reuse of the exact same snapshot name) when all of the following conditions are true for any two `pacman` operations:
+- They occur within the same minute
+- They cover the same type of operation (_Install_, _Remove_ or _Upgrade_)
+- They cover the same list of packages
+
+For example by running `pacman -S tmux` three times within the same minute (once for an _Install_ operation and two more times for two identical _Upgrade_ operations) your system may generate the following example snapshots:
+```
+zpool/root/archlinux@pacman_2023-03-07-0116_op:inst_sev:trv_pkgs:tmux                             1.66M      -  20.7G  -
+zpool/root/archlinux@pacman_2023-03-07-0116_op:upgr_sev:trv_pkgs:tmux                             2.17M      -  20.7G  -
+```
+
+Notice that there is no third snapshot for the second identical _Upgrade_ operation as this script skipped snapshot creation.
+
+The rationale is that you're doing the exact same operation twice or more. There's most likely no reasonable expectaion that your operating system enters a different state on successive `pacman` operations so there's no need to deal with multiple snapshots capturing the same state.
+
+Your `pacman` command line output will show this like so:
+```
+:: Running pre-transaction hooks...
+(1/1) Create ZFS snapshot(s)
+[WARN] ZFS snapshot skipped (same operation exists at 2023-03-07-0116):
+[WARN]   zpool/root/archlinux@pacman_2023-03-07-0116_op:upgr_sev:trv_pkgs:tmux
+[WARN] No ZFS snapshot left to do after accounting for identical operations at 2023-03-07-0116.
+```
+
+Note that this script will not blindly skip doing **_all_** snapshots in this situation. It will still happily create snapshots that don't cause naming collisions for example when affected snapshots were already deleted or when you're adding an additional dataset to the list of datasets you want to snapshot. In `pacman` command line output you'll then see warnings as needed and regular info-level messages for newly created snapshots where possible:
+```
+:: Running pre-transaction hooks...
+(1/1) Create ZFS snapshot(s)
+[WARN] ZFS snapshot skipped (same operation exists at 2023-03-07-0116):
+[WARN]   zpool/root/archlinux@pacman_2023-03-07-0116_op:upgr_sev:trv_pkgs:tmux
+[WARN]   zpool/root/archlinux/pacman-cache@pacman_2023-03-07-0116_op:upgr_sev:trv_pkgs:tmux
+[INFO] ZFS snapshot atomically done:
+[INFO]   zpool/data/var/lib/docker@pacman_2023-03-07-0116_op:upgr_sev:trv_pkgs:tmux
+```
+
+This behavior is not configurable. During testing and development we considered adding a monotonically increasing counter to timestamps such as:
+```
+...2023-03-07-0116-1...
+...2023-03-07-0116-2...
+...2023-03-07-0116-3...
+```
+While this would effectively avoid naming collisions we decided against it. Weighing pros and cons the _skip_ approach seems ever so sleightly simpler than the _counter_ approach.
+
 # Rollback
 
 After a rollback for example via the excellent [ZFSBootMenu](https://docs.zfsbootmenu.org/) `pacman` and all AUR helpers you may be using will consider the `pacman` database to be locked. No `pacman` transactions can start, you will for example see:

pacman-zfs-snapshot.sh

@@ -235,6 +235,31 @@ function trim_pkg_list_oneline () {
     trimmed_pkg_list_oneline="${shorter_pkg_list}"
 }
 
+function omit_duplicate_snaps () {
+    local existing_snaps
+    local -a unneeded_snaps
+    existing_snaps="$(zfs list -t all -oname -H)"
+
+    for planned_snap in "${planned_snaps[@]}"; do
+        if grep -Piq -- '^'"${planned_snap}"'$' <<<"${existing_snaps}"; then
+            unneeded_snaps+=("${planned_snap}")
+        else
+            needed_snaps+=("${planned_snap}")
+        fi
+    done
+
+    if [[ "${#unneeded_snaps[@]}" -gt '0' ]]; then
+        if [[ "${do_dry_run}" == 'true' ]]; then
+            pprint 'warn' 'Dry-run, ZFS snapshot skipped (same operation exists at '"${date_string}"'):'
+        else
+            pprint 'warn' 'ZFS snapshot skipped (same operation exists at '"${date_string}"'):'
+        fi
+        for unneeded_snap in "${unneeded_snaps[@]}"; do
+            pprint 'warn' '  '"${unneeded_snap}"
+        done
+    fi
+}
+
 function do_snaps () {
     local snap_name snap_return_code
     local -a planned_snaps
@@ -249,25 +274,35 @@ function do_snaps () {
         fi
         planned_snaps["${snappable_dataset_id}"]="${snap_name}"
     done
-    if [[ "${do_dry_run}" == 'true' ]]; then
+    local -a needed_snaps
-        pprint 'info' 'Dry-run, pretending to atomically do zfs snapshot:'
+    omit_duplicate_snaps
-        for planned_snap in "${planned_snaps[@]}"; do
+    if [[ "${#needed_snaps[@]}" -gt '0' ]]; then
-            pprint 'info' '  '"${planned_snap}"
+        if [[ "${do_dry_run}" == 'true' ]]; then
-        done
+            pprint 'info' 'Dry-run, pretending to atomically do ZFS snapshot:'
-    else
+            for needed_snap in "${needed_snaps[@]}"; do
-        zfs snapshot "${planned_snaps[@]}"
+                pprint 'info' '  '"${needed_snap}"
-        snap_return_code="${?}"
-        if [[ "${snap_return_code}" -eq '0' ]]; then
-            successfully_snapped_datasets=("${snappable_datasets[@]}")
-            pprint 'info' 'zfs snapshot atomically done:'
-            for planned_snap in "${planned_snaps[@]}"; do
-                pprint 'info' '  '"${planned_snap}"
             done
         else
-            pprint 'warn' 'zfs snapshot failed:'
+            zfs snapshot "${needed_snaps[@]}"
-            for planned_snap in "${planned_snaps[@]}"; do
+            snap_return_code="${?}"
-                pprint 'warn' '  '"${planned_snap}"
+            if [[ "${snap_return_code}" -eq '0' ]]; then
-            done
+                successfully_snapped_datasets=("${snappable_datasets[@]}")
+                pprint 'info' 'ZFS snapshot atomically done:'
+                for needed_snap in "${needed_snaps[@]}"; do
+                    pprint 'info' '  '"${needed_snap}"
+                done
+            else
+                pprint 'warn' 'ZFS snapshot failed:'
+                for needed_snap in "${needed_snaps[@]}"; do
+                    pprint 'warn' '  '"${needed_snap}"
+                done
+            fi
+        fi
+    else
+        if [[ "${do_dry_run}" == 'true' ]]; then
+            pprint 'warn' 'Dry-run, no ZFS snapshot left to do after accounting for identical operations at '"${date_string}"'.'
+        else
+            pprint 'warn' 'No ZFS snapshot left to do after accounting for identical operations at '"${date_string}"'.'
         fi
     fi
 }