Revert "DROP THIS don't run tests in CI"

This reverts commit a3c6173b85dede625f51ca7172a53a5aff024fae.

.circleci/config.yml

@@ -59,6 +59,7 @@ jobs:
       - run: ./lazy.sh devsetup
 
       - run: make zrepl-bin
+      - run: make test-platform-bin
       - run: make vet
       - run: make lint
       - run: make release
@@ -85,6 +86,7 @@ jobs:
             mc config host add --api s3v4 zrepl-minio https://minio.cschwarz.com ${MINIO_ACCESS_KEY} ${MINIO_SECRET_KEY}
             
             # Upload artifacts
+            mkdir -p artifacts/release
             echo "$CIRCLE_BUILD_URL" > ./artifacts/release/cirlceci_build_url
             mc cp -r artifacts/release "zrepl-minio/zrepl-ci-artifacts/${CIRCLE_SHA1}/${CIRCLE_JOB}/"
 
@@ -98,6 +100,30 @@ jobs:
               -X POST \
               -d '{"context":"zrepl/publish-ci-artifacts", "state": "success", "description":"CI Build Artifacts for '"$JOB_NAME"'", "target_url":"https://minio.cschwarz.com/minio/zrepl-ci-artifacts/'"$COMMIT"'/"}'
 
+      - run:
+          shell: /bin/bash -eo pipefail
+          when: always
+          command: |
+            if [ -n "$CIRCLE_PR_NUMBER" ]; then # CIRCLE_PR_NUMBER is guaranteed to be only present in forked PRs (external)
+              echo "Forked PR detected. Sry, can't trust you with credentials to external artifact store, use CircleCI's instead."
+              exit 0
+            fi
+            set -u # from now on
+
+            # Trigger Platform Tests
+            # ZREPL_BOT_GIT_PRWN_EU_ACCESS_TOKEN from secrets
+            ## Sync this commit to the GitLab
+            git remote add prwn-gitlab "https://zrepl-bot:$ZREPL_BOT_GIT_PRWN_EU_ACCESS_TOKEN@git.prwn.eu/problame/zrepl.git"
+            REMOTE_REF=circleci/$CIRCLE_BRANCH/$CIRCLE_BUILD_NUM
+            git push -f prwn-gitlab "HEAD:$REMOTE_REF"
+            ## Trigger Tests
+            curl -v --request POST \
+              --header "PRIVATE-TOKEN: $ZREPL_BOT_GIT_PRWN_EU_ACCESS_TOKEN" \
+              --header "Content-Type: application/json" \
+              --data '{ "ref":"'"$REMOTE_REF"'", "variables": [ {"key":"ZREPL_MAIN_REPO_COMMIT", "value": "'"$CIRCLE_SHA1"'" }, { "key":"ZREPL_CI_BUILDJOB", "value": "'"$CIRCLE_JOB"'" } ] } ' \
+              "https://git.prwn.eu/api/v4/projects/13/pipeline"
+
+
       - run:
           shell: /bin/bash -euo pipefail
           command: |

.gitlab-ci.yml

@@ -0,0 +1,17 @@
+build-and-run-platformtests:
+  variables:
+    GIT_STRATEGY: none
+  script:
+    - mkdir artifacts
+    # platformtests are installed by the CI environment
+    - /opt/platformtest-linux-amd64
+      -test.coverprofile artifacts/platformtest.cover
+      -test.v
+      __DEVEL--i-heard-you-like-tests
+      -poolname zreplplatformtest
+      -imagepath /tmp/zreplplatformtest.pool.img
+      -mountpoint /tmp/zreplplatformtest.pool
+  artifacts:
+    paths:
+      - artifacts
+  image: "images:ubuntu/focal/cloud"

.platformtest-ci/download_ci_file.bash

@@ -0,0 +1,16 @@
+#!/usr/bin/env bash
+set -euo pipefail
+cd "$(dirname "${BASH_SOURCE[0]}")"
+
+commit="$1"
+build="$2"
+src="$3"
+dst="$4"
+
+[ -n "$commit" -a -n "$build" -a -n "$src" -a -n "$dst" ] || ( echo "arguments must not be empty"; exit 1 )
+
+aws="aws --endpoint-url https://minio.cschwarz.com --no-sign-request"
+
+
+path="s3://zrepl-ci-artifacts/$commit/$build/$src"
+$aws s3 cp "$path" "$dst"

config/config.go

@@ -76,7 +76,12 @@ type SnapJob struct {
 }
 
 type SendOptions struct {
-	Encrypted bool `yaml:"encrypted"`
+	Encrypted bool                 `yaml:"encrypted"`
+	StepHolds SendOptionsStepHolds `yaml:"step_holds,optional"`
+}
+
+type SendOptionsStepHolds struct {
+	DisableIncremental bool `yaml:"disable_incremental,optional"`
 }
 
 var _ yaml.Defaulter = (*SendOptions)(nil)

daemon/control.go

@@ -120,7 +120,13 @@ func (j *controlJob) Run(ctx context.Context) {
 		jsonResponder{log, func() (interface{}, error) {
 			jobs := j.jobs.status()
 			globalZFS := zfscmd.GetReport()
-			s := Status{Jobs: jobs, Global: GlobalStatus{ZFSCmds: globalZFS}}
+			envconstReport := envconst.GetReport()
+			s := Status{
+				Jobs: jobs,
+				Global: GlobalStatus{
+					ZFSCmds:  globalZFS,
+					Envconst: envconstReport,
+				}}
 			return s, nil
 		}})
 

daemon/daemon.go

@@ -14,6 +14,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/zrepl/zrepl/daemon/logging/trace"
 	"github.com/zrepl/zrepl/endpoint"
+	"github.com/zrepl/zrepl/util/envconst"
 
 	"github.com/zrepl/zrepl/config"
 	"github.com/zrepl/zrepl/daemon/job"
@@ -150,7 +151,8 @@ type Status struct {
 }
 
 type GlobalStatus struct {
-	ZFSCmds *zfscmd.Report
+	ZFSCmds  *zfscmd.Report
+	Envconst *envconst.Report
 }
 
 func (s *jobs) status() map[string]*job.Status {

daemon/job/active.go

@@ -152,9 +152,10 @@ func modePushFromConfig(g *config.Global, in *config.PushJob, jobID endpoint.Job
 	}
 
 	m.senderConfig = &endpoint.SenderConfig{
-		FSF:     fsf,
-		Encrypt: &zfs.NilBool{B: in.Send.Encrypted},
-		JobID:   jobID,
+		FSF:                         fsf,
+		Encrypt:                     &zfs.NilBool{B: in.Send.Encrypted},
+		DisableIncrementalStepHolds: in.Send.StepHolds.DisableIncremental,
+		JobID:                       jobID,
 	}
 	m.plannerPolicy = &logic.PlannerPolicy{
 		EncryptedSend: logic.TriFromBool(in.Send.Encrypted),

daemon/job/passive.go

@@ -79,9 +79,10 @@ func modeSourceFromConfig(g *config.Global, in *config.SourceJob, jobID endpoint
 		return nil, errors.Wrap(err, "cannot build filesystem filter")
 	}
 	m.senderConfig = &endpoint.SenderConfig{
-		FSF:     fsf,
-		Encrypt: &zfs.NilBool{B: in.Send.Encrypted},
-		JobID:   jobID,
+		FSF:                         fsf,
+		Encrypt:                     &zfs.NilBool{B: in.Send.Encrypted},
+		DisableIncrementalStepHolds: in.Send.StepHolds.DisableIncremental,
+		JobID:                       jobID,
 	}
 
 	if m.snapper, err = snapper.FromConfig(g, fsf, in.Snapshotting); err != nil {

daemon/job/snapjob.go

@@ -175,6 +175,8 @@ func (j *SnapJob) doPrune(ctx context.Context) {
 		FSF:   j.fsfilter,
 		// FIXME encryption setting is irrelevant for SnapJob because the endpoint is only used as pruner.Target
 		Encrypt: &zfs.NilBool{B: true},
+		// FIXME DisableIncrementalStepHolds setting is irrelevant for SnapJob because the endpoint is only used as pruner.Target
+		DisableIncrementalStepHolds: false,
 	})
 	j.pruner = j.prunerFactory.BuildLocalPruner(ctx, sender, alwaysUpToDateReplicationCursorHistory{sender})
 	log.Info("start pruning")

docs/changelog.rst

@@ -35,18 +35,23 @@ This is a big one! Headlining features:
 * **Resumable Send & Recv Support**
   No knobs required, automatically used where supported.
 * **Hold-Protected Send & Recv**
-  Automatic ZFS holds to ensure that we can always resume a replication step.
-* **Encrypted Send & Recv Support** for OpenZFS native encryption.
-  :ref:`Configurable <job-send-options>` at the job level, i.e., for all filesystems a job is responsible for.
+  Automatic ZFS holds to ensure that we can always use resumable send&recv for a replication step.
+* **Encrypted Send & Recv Support** for OpenZFS native encryption,
+  :ref:`configurable <job-send-options>` at the job level, i.e., for all filesystems a job is responsible for.
 * **Receive-side hold on last received dataset**
   The counterpart to the replication cursor bookmark on the send-side.
   Ensures that incremental replication will always be possible between a sender and receiver.
 
-Actual changelog:
+.. TIP::
 
-* |break_config| **more restrictive job names than in prior zrepl versions**
-  Starting with this version, job names are going to be embedded into ZFS holds and bookmark names.
+   We highly recommend studying the :ref:`overview section of the configuration chapter <overview-how-replication-works>` to understand how replication works.
 
+Additional changelog:
+
+* |break| |break_config| **more restrictive job names than in prior zrepl versions**
+  Starting with this version, job names are going to be embedded into ZFS holds and bookmark names (see :ref:`here<replication-cursor-and-last-received-hold>` and :ref:`here<step-holds-and-bookmarks>`).
+  Therefore you might need to adjust your job names.
+  **Note that jobs** :issue:`cannot be renamed easily<327>` **once you start using zrepl 0.3.**
 * |break| |mig| replication cursor representation changed
 
   * zrepl now manages the :ref:`replication cursor bookmark <replication-cursor-and-last-received-hold>` per job-filesystem tuple instead of a single replication cursor per filesystem.
@@ -56,16 +61,29 @@ Actual changelog:
   * Run ``zrepl migrate replication-cursor:v1-v2`` to safely destroy old-format cursors.
     The migration will ensure that only those old-format cursors are destroyed that have been superseeded by new-format cursors.
 
+* |feature| New option ``listen_freebind`` (tcp, tls, prometheus listener)
+* |feature| :issue:`<265>` transport/tcp: support for CIDR masks in client IP whitelist
+* |feature| documented subcommand to generate ``bash`` and ``zsh`` completions
+* |feature| :issue:`<307>` ``chrome://trace`` -compatible activity tracing of zrepl daemon activity
+* |feature| logging: trace IDs for better log entry correlation with concurrent replication jobs
+* |feature| experimental environment variable for parallel replication (see :issue:`<306>` )
 * |bugfix| missing logger context vars in control connection handlers
 * |bugfix| improved error messages on ``zfs send`` errors
-* |feature| New option ``listen_freebind`` (tcp, tls, prometheus listener)
 * |bugfix| |docs| snapshotting: clarify sync-up behavior and warn about filesystems
+* |bugfix| transport/ssh: do not leak zombie ssh process on connection failures
   that will not be snapshotted until the sync-up phase is over
+* |docs| Installation: :ref:`FreeBSD jail with iocage <installation-freebsd-jail-with-iocage>`
 * |docs| Document new replication features in the :ref:`config overview <overview-how-replication-works>` and :repomasterlink:`replication/design.md`.
-* |feature| documented subcommand to generate ``bash`` and ``zsh`` completions
 * **[MAINTAINER NOTICE]** New platform tests in this version, please make sure you run them for your distro!
 * **[MAINTAINER NOTICE]** Please add the shell completions to the zrepl packages.
 
+.. NOTE::
+   |  zrepl is a spare-time project primarily developed by `Christian Schwarz <https://cschwarz.com>`_.
+   |  You can support maintenance and feature development through one of the following services:
+   |  |Donate via Patreon| |Donate via Liberapay| |Donate via PayPal|
+   |  Note that PayPal processing fees are relatively high for small donations.
+   |  For SEPA wire transfer and **commercial support**, please `contact Christian directly <https://cschwarz.com>`_.
+
 0.2.1
 -----
 
@@ -102,14 +120,6 @@ Actual changelog:
   The pool name ``zreplplatformtest`` is reserved for this use case.
   Only run ``make platformtest`` on test systems, e.g. a FreeBSD VM image.
 
-.. NOTE::
-   |  zrepl is a spare-time project primarily developed by `Christian Schwarz <https://cschwarz.com>`_.
-   |  You can support maintenance and feature development through one of the following services:
-   |  |Donate via Patreon| |Donate via Liberapay| |Donate via PayPal|
-   |  Note that PayPal processing fees are relatively high for small donations.
-   |  For SEPA wire transfer and **commercial support**, please `contact Christian directly <https://cschwarz.com>`_.
-
-
 0.1.1
 -----
 

docs/configuration/jobs.rst

@@ -19,7 +19,7 @@ Job Type ``push``
     * - ``type``
       - = ``push``
     * - ``name``
-      - unique name of the job
+      - unique name of the job :issue:`(must not change)<327>`
     * - ``connect``
       - |connect-transport|
     * - ``filesystems``
@@ -47,7 +47,7 @@ Job Type ``sink``
     * - ``type``
       - = ``sink``
     * - ``name``
-      - unique name of the job
+      - unique name of the job :issue:`(must not change)<327>`
     * - ``serve``
       - |serve-transport|
     * - ``root_fs``
@@ -70,7 +70,7 @@ Job Type ``pull``
     * - ``type``
       - = ``pull``
     * - ``name``
-      - unique name of the job
+      - unique name of the job :issue:`(must not change)<327>`
     * - ``connect``
       - |connect-transport|
     * - ``root_fs``
@@ -98,7 +98,7 @@ Job Type ``source``
     * - ``type``
       - = ``source``
     * - ``name``
-      - unique name of the job
+      - unique name of the job :issue:`(must not change)<327>`
     * - ``serve``
       - |serve-transport|
     * - ``filesystems``
@@ -137,7 +137,7 @@ Job type that only takes snapshots and performs pruning on the local machine.
     * - ``type``
       - = ``snap``
     * - ``name``
-      - unique name of the job
+      - unique name of the job :issue:`(must not change)<327>`
     * - ``filesystems``
       - |filter-spec| for filesystems to be snapshotted
     * - ``snapshotting``

docs/configuration/overview.rst

@@ -39,6 +39,10 @@ A *job* is the unit of activity tracked by the zrepl daemon.
 The ``type`` of a job determines its role in a replication setup and in snapshot management.
 Jobs are identified by their ``name``, both in log files and the ``zrepl status`` command.
 
+.. NOTE::
+   The job name is persisted in several places on disk and thus :issue:`cannot be changed easily<327>`.
+
+
 Replication always happens between a pair of jobs: one is the **active side**, and one the **passive side**.
 The active side connects to the passive side using a :ref:`transport <transport>` and starts executing the replication logic.
 The passive side responds to requests from the active side after checking its permissions.
@@ -99,7 +103,7 @@ One of the major design goals of the replication module is to avoid any duplicat
 As such, the code works on abstract senders and receiver **endpoints**, where typically one will be implemented by a local program object and the other is an RPC client instance.
 Regardless of push- or pull-style setup, the logic executes on the active side, i.e. in the ``push`` or ``pull`` job.
 
-The following steps take place during replication and can be monitored using the ``zrepl status`` subcommand:
+The following high-level steps take place during replication and can be monitored using the ``zrepl status`` subcommand:
 
 * Plan the replication:
 
@@ -107,9 +111,9 @@ The following steps take place during replication and can be monitored using the
   * Build the **replication plan**
 
     * Per filesystem, compute a diff between sender and receiver snapshots
-    * Build a list of replication steps
+    * Build a list of **replication steps**
 
-      * If possible, use incremental and resumable sends (``zfs send -i``)
+      * If possible, use incremental and resumable sends
       * Otherwise, use full send of most recent snapshot on sender
 
   * Retry on errors that are likely temporary (i.e. network failures).
@@ -128,16 +132,8 @@ The following steps take place during replication and can be monitored using the
    
 The idea behind the execution order of replication steps is that if the sender snapshots all filesystems simultaneously at fixed intervals, the receiver will have all filesystems snapshotted at time ``T1`` before the first snapshot at ``T2 = T1 + $interval`` is replicated.
 
-.. _replication-cursor-and-last-received-hold:
-
-**Replication cursor** bookmark and **last-received-hold** are managed by zrepl to ensure that future replications can always be done incrementally:
-the replication cursor is a send-side bookmark of the most recent successfully replicated snapshot,
-and the last-received-hold is a hold of that snapshot on the receiving side.
-The replication cursor has the format ``#zrepl_CUSOR_G_<GUID>_J_<JOBNAME>``.
-The last-received-hold tag has the format ``#zrepl_last_received_J_<JOBNAME>``.
-Encoding the job name in the names ensures that multiple sending jobs can replicate the same filesystem to different receivers without interference.
-The ``zrepl holds list`` provides a listing of all bookmarks and holds managed by zrepl.
-
+Placeholder Filesystems
+^^^^^^^^^^^^^^^^^^^^^^^
 .. _replication-placeholder-property:
 
 **Placeholder filesystems** on the receiving side are regular ZFS filesystems with the placeholder property ``zrepl:placeholder=on``.
@@ -148,16 +144,83 @@ Thus, zrepl creates the parent filesystems as placeholders on the receiving side
 If at some point ``S/H`` and ``S`` shall be replicated, the receiving side invalidates the placeholder flag automatically.
 The ``zrepl test placeholder`` command can be used to check whether a filesystem is a placeholder.
 
+ZFS Background Knowledge
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+This section gives some background knowledge about ZFS features that zrepl uses to guarantee that
+**incremental replication is always possible and that started replication steps can always be resumed if they are interrupted.**
+
+**ZFS Send Modes & Bookmarks**
+ZFS supports full sends (``zfs send fs@to``) and incremental sends (``zfs send -i @from fs@to``).
+Full sends are used to create a new filesystem on the receiver with the send-side state of ``fs@to``.
+Incremental sends only transfer the delta between ``@from`` and ``@to``.
+Incremental sends require that ``@from`` be present on the receiving side when receiving the incremental stream.
+Incremental sends can also use a ZFS bookmark as *from* on the sending side (``zfs send -i #bm_from fs@to``), where ``#bm_from`` was created using ``zfs bookmark fs@from fs#bm_from``.
+The receiving side must always have the actual snapshot ``@from``, regardless of whether the sending side uses ``@from`` or a bookmark of it.
+
+**Resumable Send & Recv**
+The ``-s`` flag for ``zfs recv`` tells zfs to save the partially received send stream in case it is interrupted.
+To resume the replication, the receiving side filesystem's ``receive_resume_token`` must be passed to a new ``zfs send -t <value> | zfs recv`` command.
+A full send can only be resumed if ``@to`` still exists.
+An incremental send can only be resumed if ``@to`` still exists *and* either ``@from`` still exists *or* a bookmark ``#fbm`` of ``@from`` still exists.
+
+**ZFS Holds**
+ZFS holds prevent a snapshot from being deleted through ``zfs destroy``, letting the destroy fail with a ``datset is busy`` error.
+Holds are created and referred to by a user-defined *tag*. They can be thought of as a named, persistent lock on the snapshot.
+
+.. _replication-cursor-and-last-received-hold:
+
+Guaranteeing That Incremental Sends Are Always Possible
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+**Replication cursor** bookmark and **last-received-hold** are managed by zrepl to ensure that future replications can always be done incrementally.
+The replication cursor is a send-side bookmark of the most recent successfully replicated snapshot,
+and the last-received-hold is a hold of that snapshot on the receiving side.
+Both are moved aomically after the receiving side has confirmed that a replication step is complete.
+
+The replication cursor has the format ``#zrepl_CUSOR_G_<GUID>_J_<JOBNAME>``.
+The last-received-hold tag has the format ``zrepl_last_received_J_<JOBNAME>``.
+Encoding the job name in the names ensures that multiple sending jobs can replicate the same filesystem to different receivers without interference.
+
+The ``zrepl zfs-abstraction list`` command provides a listing of all bookmarks and holds managed by zrepl.
+
+.. _step-holds-and-bookmarks:
+
+Guaranteeing That Sends Are Always Resumable
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+**Step holds** are zfs holds managed by zrepl to ensure that a replication step can always be resumed if it is interrupted, e.g., due to network outage.
+zrepl creates step holds before it attempts a replication step and releases them after the receiver confirms that the replication step is complete.
+For an initial replication ``full @initial_snap``, zrepl puts a zfs hold on ``@initial_snap``.
+For an incremental send ``@from -> @to``, zrepl puts a zfs hold on both ``@from`` and ``@to``.
+Note that ``@from`` is not strictly necessary for resumability -- a bookmark on the sending side would be sufficient --, but size-estimation in currently used OpenZFS versions only works if ``@from`` is a snapshot.
+
+The hold tag has the format ``zrepl_STEP_J_<JOBNAME>``.
+A job only ever has one active send per filesystem.
+Thus, there are never more than two step holds for a given pair of ``(job,filesystem)``.
+
+**Step bookmarks** are zrepl's equivalent for holds on bookmarks (ZFS does not support putting holds on bookmarks).
+They are intended for a situation where a replication step uses a bookmark ``#bm`` as incremental ``from`` that is not managed by zrepl.
+To ensure resumability, zrepl copies ``#bm`` to step bookmark ``#zrepl_STEP_G_<GUID>_J_<JOBNAME>``.
+If the replication is interrupted and ``#bm`` is deleted by the user, the step bookmark remains as an incremental source for the resumable send.
+Note that zrepl does not yet support creating step bookmarks because the `corresponding ZFS feature for copying bookmarks <https://github.com/openzfs/zfs/pull/9571>`_ is not yet widely available .
+Subscribe to zrepl :issue:`326` for details.
+
+The ``zrepl zfs-abstraction list`` command provides a listing of all bookmarks and holds managed by zrepl.
+
+.. NOTE::
+
+    More details can be found in the design document :repomasterlink:`replication/design.md`.
+
+Limitations
+^^^^^^^^^^^
+
 .. ATTENTION::
 
     Currently, zrepl does not replicate filesystem properties.
     When receiving a filesystem, it is never mounted (`-u` flag)  and `mountpoint=none` is set.
     This is temporary and being worked on :issue:`24`.
 
-.. NOTE::
-
-    More details can be found in the design document :repomasterlink:`replication/design.md`.
-
 
 .. _jobs-multiple-jobs:
 
@@ -177,7 +240,7 @@ Multiple Jobs & More than 2 Machines
 If you would like to see improvements to multi-job setups, please `open an issue on GitHub <https://github.com/zrepl/zrepl/issues/new>`_.
 
 No Overlapping
-~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^
 
 Jobs run independently of each other.
 If two jobs match the same filesystem with their ``filesystems`` filter, they will operate on that filesystem independently and potentially in parallel.
@@ -185,14 +248,14 @@ For example, if job A prunes snapshots that job B is planning to replicate, the
 However, the next replication attempt will re-examine the situation from scratch and should work.
 
 N push jobs to 1 sink
-~~~~~~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^^^^^^
 
 The :ref:`sink job <job-sink>` namespaces by client identity.
 It is thus safe to push to one sink job with different client identities.
 If the push jobs have the same client identity, the filesystems matched by the push jobs must be disjoint to avoid races.
 
 N pull jobs from 1 source
-~~~~~~~~~~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Multiple pull jobs pulling from the same source have potential for race conditions during pruning:
 each pull job prunes the source side independently, causing replication-prune and prune-prune races.

docs/configuration/sendrecvoptions.rst

@@ -16,6 +16,8 @@ Send Options
      filesystems: ...
      send:
        encrypted: true
+       step_holds:
+         disable_incremental: false
      ...
 
 :ref:`Source<job-source>` and :ref:`push<job-push>` jobs have an optional ``send`` configuration section.
@@ -34,6 +36,17 @@ Filesystems matched by ``filesystems`` that are not encrypted are not sent and w
 
 If ``encryption=false``, zrepl expects that filesystems matching ``filesystems`` are not encrypted or have loaded encryption keys.
 
+
+``step_holds.disable_incremental`` option
+-----------------------------------------
+
+The ``step_holds.disable_incremental`` variable controls whether the creation of :ref:`step holds <step-holds-and-bookmarks>` should be disabled for incremental replication.
+The default value is ``false``.
+
+Disabling step holds has the disadvantage that steps :ref:`might not be resumable <step-holds-and-bookmarks>` if interrupted.
+However, the advantage and :issue:`reason for existence <288>` of this flag is that it allows the pruner to delete snapshots of interrupted replication steps
+which is useful if replication happens so rarely (or fails so frequently) that the amount of disk space exclusively referenced by the step's snapshots becomes intolerable.
+
 .. _job-recv-options:
 
 Recv Options

endpoint/endpoint.go

@@ -21,9 +21,10 @@ import (
 )
 
 type SenderConfig struct {
-	FSF     zfs.DatasetFilter
-	Encrypt *zfs.NilBool
-	JobID   JobID
+	FSF                         zfs.DatasetFilter
+	Encrypt                     *zfs.NilBool
+	DisableIncrementalStepHolds bool
+	JobID                       JobID
 }
 
 func (c *SenderConfig) Validate() error {
@@ -39,9 +40,10 @@ func (c *SenderConfig) Validate() error {
 
 // Sender implements replication.ReplicationEndpoint for a sending side
 type Sender struct {
-	FSFilter zfs.DatasetFilter
-	encrypt  *zfs.NilBool
-	jobId    JobID
+	FSFilter                    zfs.DatasetFilter
+	encrypt                     *zfs.NilBool
+	disableIncrementalStepHolds bool
+	jobId                       JobID
 }
 
 func NewSender(conf SenderConfig) *Sender {
@@ -49,9 +51,10 @@ func NewSender(conf SenderConfig) *Sender {
 		panic("invalid config" + err.Error())
 	}
 	return &Sender{
-		FSFilter: conf.FSF,
-		encrypt:  conf.Encrypt,
-		jobId:    conf.JobID,
+		FSFilter:                    conf.FSF,
+		encrypt:                     conf.Encrypt,
+		disableIncrementalStepHolds: conf.DisableIncrementalStepHolds,
+		jobId:                       conf.JobID,
 	}
 }
 
@@ -221,9 +224,11 @@ func (s *Sender) Send(ctx context.Context, r *pdu.SendReq) (*pdu.SendRes, io.Rea
 		}
 	}
 
+	takeStepHolds := sendArgs.FromVersion == nil || !s.disableIncrementalStepHolds
+
 	var fromHold, toHold Abstraction
 	// make sure `From` doesn't go away in order to make this step resumable
-	if sendArgs.From != nil {
+	if sendArgs.From != nil && takeStepHolds {
 		fromHold, err = HoldStep(ctx, sendArgs.FS, *sendArgs.FromVersion, s.jobId) // no shadow
 		if err == zfs.ErrBookmarkCloningNotSupported {
 			getLogger(ctx).Debug("not creating step bookmark because ZFS does not support it")
@@ -232,10 +237,12 @@ func (s *Sender) Send(ctx context.Context, r *pdu.SendReq) (*pdu.SendRes, io.Rea
 			return nil, nil, errors.Wrapf(err, "cannot hold `from` version %q before starting send", *sendArgs.FromVersion)
 		}
 	}
-	// make sure `To` doesn't go away in order to make this step resumable
-	toHold, err = HoldStep(ctx, sendArgs.FS, sendArgs.ToVersion, s.jobId)
-	if err != nil {
-		return nil, nil, errors.Wrapf(err, "cannot hold `to` version %q before starting send", sendArgs.ToVersion)
+	if takeStepHolds {
+		// make sure `To` doesn't go away in order to make this step resumable
+		toHold, err = HoldStep(ctx, sendArgs.FS, sendArgs.ToVersion, s.jobId)
+		if err != nil {
+			return nil, nil, errors.Wrapf(err, "cannot hold `to` version %q before starting send", sendArgs.ToVersion)
+		}
 	}
 
 	// cleanup the mess that _this function_ might have created in prior failed attempts:
@@ -254,11 +261,11 @@ func (s *Sender) Send(ctx context.Context, r *pdu.SendReq) (*pdu.SendRes, io.Rea
 	//
 	// Note further that a resuming send, due to the idempotent nature of func CreateReplicationCursor and HoldStep,
 	// will never lose its step holds because we just (idempotently re-)created them above, before attempting the cleanup.
+	liveAbs := []Abstraction{fromHold, toHold, fromReplicationCursor}
 	func() {
 		ctx, endSpan := trace.WithSpan(ctx, "cleanup-stale-abstractions")
 		defer endSpan()
 
-		liveAbs := []Abstraction{fromHold, toHold, fromReplicationCursor}
 		keep := func(a Abstraction) (keep bool) {
 			keep = false
 			for _, k := range liveAbs {
@@ -283,13 +290,11 @@ func (s *Sender) Send(ctx context.Context, r *pdu.SendReq) (*pdu.SendRes, io.Rea
 		}
 		sendAbstractionsCacheSingleton.TryBatchDestroy(ctx, s.jobId, sendArgs.FS, keep, check)
 	}()
-
-	if fromHold != nil {
-		sendAbstractionsCacheSingleton.Put(fromHold)
-	}
-	sendAbstractionsCacheSingleton.Put(toHold)
-	if fromReplicationCursor != nil {
-		sendAbstractionsCacheSingleton.Put(fromReplicationCursor)
+	// now add the newly created abstractions to the cleaned-up cache
+	for _, a := range liveAbs {
+		if a != nil {
+			sendAbstractionsCacheSingleton.Put(a)
+		}
 	}
 
 	sendStream, err := zfs.ZFSSend(ctx, sendArgs)

go.mod

@@ -22,7 +22,7 @@ require (
 	github.com/onsi/gomega v1.7.0 // indirect
 	github.com/pkg/errors v0.8.1
 	github.com/pkg/profile v1.2.1
-	github.com/problame/go-netssh v0.0.0-20191209123953-18d8aa6923c7
+	github.com/problame/go-netssh v0.0.0-20200601114649-26439f9f0dc5
 	github.com/prometheus/client_golang v1.2.1
 	github.com/prometheus/common v0.7.0
 	github.com/sergi/go-diff v1.0.1-0.20180205163309-da645544ed44 // go1.12 thinks it needs this

go.sum

@@ -212,6 +212,8 @@ github.com/problame/go-netssh v0.0.0-20191026123024-f34099f4f6b1 h1:HH8yzlaZq/A8
 github.com/problame/go-netssh v0.0.0-20191026123024-f34099f4f6b1/go.mod h1:JRs3nyBjvOv/9YHC+Vlw9z1Jfzl5s5t0BNnTzmclj1c=
 github.com/problame/go-netssh v0.0.0-20191209123953-18d8aa6923c7 h1:Oik8tLrLhoMq4fduDRuNNOAQ+q0M0dXkjAYLUf4+mAc=
 github.com/problame/go-netssh v0.0.0-20191209123953-18d8aa6923c7/go.mod h1:Ba6RaFpK1+IHWs1wLZ6sCBuXkt4iAVLeOG5GinXHusM=
+github.com/problame/go-netssh v0.0.0-20200601114649-26439f9f0dc5 h1:1eSrO2WAeSXjMol8WRKW9LekL1252VIMaKQwWkmEdvs=
+github.com/problame/go-netssh v0.0.0-20200601114649-26439f9f0dc5/go.mod h1:Ba6RaFpK1+IHWs1wLZ6sCBuXkt4iAVLeOG5GinXHusM=
 github.com/prometheus/client_golang v0.0.0-20180410130117-e11c6ff8170b/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
 github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
 github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=

platformtest/tests/generated_cases.go

@@ -19,7 +19,8 @@ var Cases = []Case{BatchDestroy,
 	ReplicationIncrementalCleansUpStaleAbstractionsWithCacheOnSecondReplication,
 	ReplicationIncrementalCleansUpStaleAbstractionsWithoutCacheOnSecondReplication,
 	ReplicationIncrementalIsPossibleIfCommonSnapshotIsDestroyed,
-	ReplicationIsResumableFullSend,
+	ReplicationIsResumableFullSend__DisableIncrementalStepHolds_False,
+	ReplicationIsResumableFullSend__DisableIncrementalStepHolds_True,
 	ResumableRecvAndTokenHandling,
 	ResumeTokenParsing,
 	SendArgsValidationEncryptedSendOfUnencryptedDatasetForbidden,

platformtest/tests/replication.go

@@ -26,10 +26,11 @@ import (
 // of a new sender and receiver instance and one blocking invocation
 // of the replication engine without encryption
 type replicationInvocation struct {
-	sjid, rjid      endpoint.JobID
-	sfs             string
-	rfsRoot         string
-	interceptSender func(e *endpoint.Sender) logic.Sender
+	sjid, rjid                  endpoint.JobID
+	sfs                         string
+	rfsRoot                     string
+	interceptSender             func(e *endpoint.Sender) logic.Sender
+	disableIncrementalStepHolds bool
 }
 
 func (i replicationInvocation) Do(ctx *platformtest.Context) *report.Report {
@@ -42,9 +43,10 @@ func (i replicationInvocation) Do(ctx *platformtest.Context) *report.Report {
 	err := sfilter.Add(i.sfs, "ok")
 	require.NoError(ctx, err)
 	sender := i.interceptSender(endpoint.NewSender(endpoint.SenderConfig{
-		FSF:     sfilter.AsFilter(),
-		Encrypt: &zfs.NilBool{B: false},
-		JobID:   i.sjid,
+		FSF:                         sfilter.AsFilter(),
+		Encrypt:                     &zfs.NilBool{B: false},
+		DisableIncrementalStepHolds: i.disableIncrementalStepHolds,
+		JobID:                       i.sjid,
 	}))
 	receiver := endpoint.NewReceiver(endpoint.ReceiverConfig{
 		JobID:                      i.rjid,
@@ -86,10 +88,11 @@ func ReplicationIncrementalIsPossibleIfCommonSnapshotIsDestroyed(ctx *platformte
 	snap1 := fsversion(ctx, sfs, "@1")
 
 	rep := replicationInvocation{
-		sjid:    sjid,
-		rjid:    rjid,
-		sfs:     sfs,
-		rfsRoot: rfsRoot,
+		sjid:                        sjid,
+		rjid:                        rjid,
+		sfs:                         sfs,
+		rfsRoot:                     rfsRoot,
+		disableIncrementalStepHolds: false,
 	}
 	rfs := rep.ReceiveSideFilesystem()
 
@@ -149,10 +152,11 @@ func implReplicationIncrementalCleansUpStaleAbstractions(ctx *platformtest.Conte
 	rfsRoot := ctx.RootDataset + "/receiver"
 
 	rep := replicationInvocation{
-		sjid:    sjid,
-		rjid:    rjid,
-		sfs:     sfs,
-		rfsRoot: rfsRoot,
+		sjid:                        sjid,
+		rjid:                        rjid,
+		sfs:                         sfs,
+		rfsRoot:                     rfsRoot,
+		disableIncrementalStepHolds: false,
 	}
 	rfs := rep.ReceiveSideFilesystem()
 
@@ -322,7 +326,15 @@ func (s *PartialSender) Send(ctx context.Context, r *pdu.SendReq) (r1 *pdu.SendR
 	return r1, r2, r3
 }
 
-func ReplicationIsResumableFullSend(ctx *platformtest.Context) {
+func ReplicationIsResumableFullSend__DisableIncrementalStepHolds_False(ctx *platformtest.Context) {
+	implReplicationIsResumableFullSend(ctx, false)
+}
+
+func ReplicationIsResumableFullSend__DisableIncrementalStepHolds_True(ctx *platformtest.Context) {
+	implReplicationIsResumableFullSend(ctx, true)
+}
+
+func implReplicationIsResumableFullSend(ctx *platformtest.Context, disableIncrementalStepHolds bool) {
 
 	platformtest.Run(ctx, platformtest.PanicErr, ctx.RootDataset, `
 		CREATEROOT
@@ -353,6 +365,7 @@ func ReplicationIsResumableFullSend(ctx *platformtest.Context) {
 		interceptSender: func(e *endpoint.Sender) logic.Sender {
 			return &PartialSender{Sender: e, failAfterByteCount: 1 << 20}
 		},
+		disableIncrementalStepHolds: disableIncrementalStepHolds,
 	}
 	rfs := rep.ReceiveSideFilesystem()
 

util/envconst/envconst.go

@@ -12,84 +12,100 @@ import (
 
 var cache sync.Map
 
-func Duration(varname string, def time.Duration) time.Duration {
+func Reset() {
+	cache.Range(func(key, _ interface{}) bool {
+		cache.Delete(key)
+		return true
+	})
+}
+
+func Duration(varname string, def time.Duration) (d time.Duration) {
+	var err error
 	if v, ok := cache.Load(varname); ok {
 		return v.(time.Duration)
 	}
 	e := os.Getenv(varname)
 	if e == "" {
-		return def
-	}
-	d, err := time.ParseDuration(e)
-	if err != nil {
-		panic(err)
+		d = def
+	} else {
+		d, err = time.ParseDuration(e)
+		if err != nil {
+			panic(err)
+		}
 	}
 	cache.Store(varname, d)
 	return d
 }
 
-func Int(varname string, def int) int {
+func Int(varname string, def int) (d int) {
 	if v, ok := cache.Load(varname); ok {
 		return v.(int)
 	}
 	e := os.Getenv(varname)
 	if e == "" {
-		return def
+		d = def
+	} else {
+		d64, err := strconv.ParseInt(e, 10, strconv.IntSize)
+		if err != nil {
+			panic(err)
+		}
+		d = int(d64)
 	}
-	d64, err := strconv.ParseInt(e, 10, strconv.IntSize)
-	if err != nil {
-		panic(err)
-	}
-	d := int(d64)
 	cache.Store(varname, d)
 	return d
 }
 
-func Int64(varname string, def int64) int64 {
+func Int64(varname string, def int64) (d int64) {
+	var err error
 	if v, ok := cache.Load(varname); ok {
 		return v.(int64)
 	}
 	e := os.Getenv(varname)
 	if e == "" {
-		return def
-	}
-	d, err := strconv.ParseInt(e, 10, 64)
-	if err != nil {
-		panic(err)
+		d = def
+	} else {
+		d, err = strconv.ParseInt(e, 10, 64)
+		if err != nil {
+			panic(err)
+		}
 	}
 	cache.Store(varname, d)
 	return d
 }
 
-func Bool(varname string, def bool) bool {
+func Bool(varname string, def bool) (d bool) {
+	var err error
 	if v, ok := cache.Load(varname); ok {
 		return v.(bool)
 	}
 	e := os.Getenv(varname)
 	if e == "" {
-		return def
-	}
-	d, err := strconv.ParseBool(e)
-	if err != nil {
-		panic(err)
+		d = def
+	} else {
+		d, err = strconv.ParseBool(e)
+		if err != nil {
+			panic(err)
+		}
 	}
 	cache.Store(varname, d)
 	return d
 }
 
-func String(varname string, def string) string {
+func String(varname string, def string) (d string) {
 	if v, ok := cache.Load(varname); ok {
 		return v.(string)
 	}
 	e := os.Getenv(varname)
 	if e == "" {
-		return def
+		d = def
+	} else {
+		d = e
 	}
-	cache.Store(varname, e)
-	return e
+	cache.Store(varname, d)
+	return d
 }
 
-func Var(varname string, def flag.Value) interface{} {
+func Var(varname string, def flag.Value) (d interface{}) {
 
 	// use def's type to instantiate a new object of that same type
 	// and call flag.Value.Set() on it
@@ -100,19 +116,43 @@ func Var(varname string, def flag.Value) interface{} {
 	defElemType := defType.Elem()
 
 	if v, ok := cache.Load(varname); ok {
-		return v.(string)
+		return v
 	}
+
 	e := os.Getenv(varname)
 	if e == "" {
-		return def
+		d = def
+	} else {
+		newInstance := reflect.New(defElemType)
+		if err := newInstance.Interface().(flag.Value).Set(e); err != nil {
+			panic(err)
+		}
+		d = newInstance.Interface()
 	}
 
-	newInstance := reflect.New(defElemType)
-	if err := newInstance.Interface().(flag.Value).Set(e); err != nil {
-		panic(err)
-	}
-
-	res := newInstance.Interface()
-	cache.Store(varname, res)
-	return res
+	cache.Store(varname, d)
+	return d
+}
+
+type Report struct {
+	Entries []EntryReport
+}
+
+type EntryReport struct {
+	Var         string
+	Value       string
+	ValueGoType string
+}
+
+func GetReport() *Report {
+	var r Report
+	cache.Range(func(key, value interface{}) bool {
+		r.Entries = append(r.Entries, EntryReport{
+			Var:         key.(string),
+			Value:       fmt.Sprintf("%v", value),
+			ValueGoType: fmt.Sprintf("%T", value),
+		})
+		return true
+	})
+	return &r
 }

util/envconst/envconst_test.go

@@ -32,19 +32,27 @@ func (m *ExampleVarType) Set(s string) error {
 
 const EnvVarName = "ZREPL_ENVCONST_UNIT_TEST_VAR"
 
-func TestVar(t *testing.T) {
+func TestVarDefaultValue(t *testing.T) {
+	envconst.Reset()
 	_, set := os.LookupEnv(EnvVarName)
 	require.False(t, set)
 	defer os.Unsetenv(EnvVarName)
 
 	val := envconst.Var(EnvVarName, &Var1)
 	if &Var1 != val {
-		t.Errorf("default value shut be same address")
+		t.Errorf("default value should be same address")
 	}
+}
+
+func TestVarOverriddenValue(t *testing.T) {
+	envconst.Reset()
+	_, set := os.LookupEnv(EnvVarName)
+	require.False(t, set)
+	defer os.Unsetenv(EnvVarName)
 
 	err := os.Setenv(EnvVarName, "var2")
 	require.NoError(t, err)
 
-	val = envconst.Var(EnvVarName, &Var1)
+	val := envconst.Var(EnvVarName, &Var1)
 	require.Equal(t, &Var2, val, "only structural identity is required for non-default vars")
 }