Update snapshot commit rules #1972

jumaffre · 2020-12-01T17:33:43Z

A snapshot now becomes committed once there has been two commits on the snapshot evidence. Waiting for a second commit guarantees that there will be a proof (i.e. signature) of the commit of the snapshot evidence in the ledger.

As snapshot file names now include the seqno at which the snapshot evidence was committed, starting up from the latest committed snapshot on join/recovery should always succeed. This is achieved by picking the latest committed snapshot whose evidence commit seqno is included in the ledger.

This should simplify #1925 and the overall safety of the join/recover from snapshot procedure, at the cost of waiting a little longer (i.e. another round of commit) for snapshots to be committed.

Note that if the service is completely idle after the snapshot evidence has been committed, the snapshot won't actually be committed until the next write transaction is committed.

Edit: The snapshot evidence seqno is now included in the snapshot file name as soon as the snapshot is serialised to disk.

…udit

eddyashton · 2020-12-01T17:49:56Z

2 slightly meta points:

I think we need to give this concept a different name than 'committed'. We already have overloaded local vs global commit (and use commit within the KV code for even earlier steps), and we want 'committed' to mean something more precise, in relation to a transaction's state within the consensus. In fact I'm not completely sure what we're waiting for here - 2 signatures? Or a signature whose global commit marker has moved past a certain point, followed by a signature whose global commit marker is past that earlier signature? "A snapshot now becomes final" or "A snapshot now becomes useable" once this state is reached, but make it clear this snapshot state is distinct from the state of any of its individual contents, or even the evidence for the snapshot which is persisted into the ledger.
This seems like a major shortcoming:

Note that if the service is completely idle after the snapshot evidence has been committed, the snapshot won't actually be committed until the next write transaction is committed.

Means we can never produce a snapshot of the service's final state, with (IUIC) an arbitrarily long suffix of transactions that are in the ledger, committed, but not snapshotted?

jumaffre · 2020-12-01T17:50:21Z

doc/operators/ledger_snapshot.rst

@@ -69,9 +69,9 @@ Once a snapshot has been generated by the primary, operators can copy or mount t

 To validate the snapshot a node is added from, the node first replays the transactions in the ledger following the snapshot until the proof that the snapshot was committed by the service to join is found. This process requires operators to copy the ledger suffix to the node's ledger directory. The validation procedure is generally quick and the node will automatically join the service one the snapshot has been validated. On recovery, the snapshot is automatically verified as part of the usual ledger recovery procedure.

-For example, if a node is added using the ``snapshot_1000.committed_1250`` snapshot file, operators should copy the ledger files containing the sequence numbers ``1000`` to ``1250`` to the directories specified by ``--ledger-dir`` (or ``--read-only ledger-dir``). This would involve copying the ledger files following the snapshot sequence number ``1000`` until the evidence sequence number ``1250``, e.g. ``ledger_1001-1200.committed`` and ``ledger_1201-1500.committed``, to the joining node's ledger directory.
+For example, if a node is added using the ``snapshot_1000.committed_1250_1300`` snapshot file, operators should copy the ledger files containing all the sequence numbers between ``1000`` to ``1300`` to the directories specified by ``--ledger-dir`` (or ``--read-only ledger-dir``). This would involve copying the ledger files following the snapshot sequence number ``1000`` until the evidence commit sequence number ``1300``, e.g. ``ledger_1001-1200.committed`` and ``ledger_1201-1500.committed``, to the joining node's ledger directory.


Note the new _<evidence_commit_seqno> suffix at the end of the snapshot file name.

jumaffre · 2020-12-01T17:52:19Z

src/host/snapshot.h

@@ -17,23 +19,12 @@ namespace asynchost
  {
  private:
    const std::string snapshot_dir;
+    const Ledger& ledger;


The host snapshot manager now takes a reference to the ledger so that it can verify that the latest committed snapshot has a proof of evidence in the ledger.

ghost · 2020-12-01T18:00:34Z

snapshot_commit_audit@16251 aka 20201203.20 vs master ewma over 50 builds from 15498 to 16236

jumaffre · 2020-12-02T10:32:39Z

@eddyashton: good points!

I think we need to give this concept a different name than 'committed'. We already have overloaded local vs global commit (and use commit within the KV code for even earlier steps), and we want 'committed' to mean something more precise, in relation to a transaction's state within the consensus. In fact I'm not completely sure what we're waiting for here - 2 signatures? Or a signature whose global commit marker has moved past a certain point, followed by a signature whose global commit marker is past that earlier signature? "A snapshot now becomes final" or "A snapshot now becomes useable" once this state is reached, but make it clear this snapshot state is distinct from the state of any of its individual contents, or even the evidence for the snapshot which is persisted into the ledger.

That's true. However, from an operator's point of view, we already use "committed" for a number of things, e.g. ledger files (and we use that word loosely there, as a non-committed ledger file probably contains committed entries). We may want to change the wording in the code, but I'm not too keen on creating a new term for operators.

What we're waiting for here is a signature in a ledger which confirms that the (global) commit point passed the snapshot evidence seqno. This is what nodes that join/recover from the ledger check for to verify the validity of the ledger (explained here). We achieve this by waiting for two commits on the snapshot evidence: the first commit proves that the snapshot evidence was committed, the second commit proves that a signature that contains (at least) the first commit seqno was created.

This seems like a major shortcoming:

Note that if the service is completely idle after the snapshot evidence has been committed, the snapshot won't actually be committed until the next write transaction is committed.

Means we can never produce a snapshot of the service's final state, with (IUIC) an arbitrarily long suffix of transactions that are in the ledger, committed, but not snapshotted?

This is already the case in #1925 but #1925 only is even worse: we mark snapshots are committed (i.e. usable) as soon as the evidence is committed, which means that a node may join/recover from a snapshot which isn't valid. This PR (#1972) delays the moment a snapshot is usable but we can then select early (on the host side) whether the snapshot we join/recover from will be able to be validated (provided that ledger integrity is verified).

I think this is a general limitation of our signatures: there never is a signature that validates the latest commit seqno (we only generate signatures based on time if the latest entry wasn't a signature).

…snapshot_commit_audit

jumaffre · 2020-12-03T17:02:31Z

tests/reconfiguration.py

@@ -170,14 +170,6 @@ def run(args):
        if args.snapshot_tx_interval is not None:
            test_add_node_from_snapshot(network, args, copy_ledger_read_only=True)

-            try:


Removed for now as the test will succeed until #1925 is merged.

…snapshot_commit_audit

jumaffre · 2020-12-03T17:08:25Z

tests/infra/network.py

@@ -762,17 +762,71 @@ def wait_for_new_primary(self, old_primary_id, timeout_multiplier=2):
        flush_info(logs, None)
        raise error(f"A new primary was not elected after {timeout} seconds")

-    def wait_for_snapshot_committed_for(self, seqno, timeout=3):
-        primary, _ = self.find_primary()
+    def wait_for_commit_proof(self, node, seqno, timeout=3):


Changes in this file are a little awkward but they help us to guarantee that historical queries after a node has joined from a snapshot work. In such scenario, we really want the new node to join from the latest available snapshot. If it didn't verifying the historical query would be trivial as the node would simply have received the historical ledger through classic catch up.

This should hopefully get simpler once snapshots can be trivially verified from a receipt over their evidence.

Julien Maffre added 8 commits December 1, 2020 10:55

Snapshot is committed only when evidence proof is committed

04c6865

Include evidence commit idx on snapshot file name

ea3500d

Host only picks snapshot with committed evidence

549b2cb

Merge remote-tracking branch 'upstream/master' into snapshot_commit_a…

d222aa9

…udit

Docs

f682c0a

from_chars

cffaf59

Revert change to infra

5306c04

Cleanup before PR

d0d1acd

jumaffre requested a review from a team as a code owner December 1, 2020 17:33

jumaffre commented Dec 1, 2020

View reviewed changes

Merge branch 'master' into snapshot_commit_audit

9e04000

Julien Maffre and others added 7 commits December 2, 2020 11:26

Fix build

8207787

Merge branch 'snapshot_commit_audit' of github.com:jumaffre/CCF into …

d7368a4

…snapshot_commit_audit

Merge branch 'master' into snapshot_commit_audit

7a0d102

WIP: infra handling of new snapshot commit scheme

7954426

Reconfiguration test works

c0e84d8

Update docs

8f691c8

Merge branch 'master' into snapshot_commit_audit

e306f35

jumaffre commented Dec 3, 2020

View reviewed changes

Julien Maffre added 2 commits December 3, 2020 17:03

Oops

1b12644

Merge branch 'snapshot_commit_audit' of github.com:jumaffre/CCF into …

163545b

…snapshot_commit_audit

jumaffre commented Dec 3, 2020

View reviewed changes

Import

71b2fc6

achamayou approved these changes Dec 3, 2020

View reviewed changes

achamayou merged commit 489e511 into microsoft:master Dec 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update snapshot commit rules #1972

Update snapshot commit rules #1972

jumaffre commented Dec 1, 2020 •

edited

Loading

eddyashton commented Dec 1, 2020

jumaffre Dec 1, 2020

jumaffre Dec 1, 2020

ghost commented Dec 1, 2020 •

edited by ghost

Loading

jumaffre commented Dec 2, 2020

jumaffre Dec 3, 2020

jumaffre Dec 3, 2020

jumaffre Dec 3, 2020

Update snapshot commit rules #1972

Update snapshot commit rules #1972

Conversation

jumaffre commented Dec 1, 2020 • edited Loading

eddyashton commented Dec 1, 2020

jumaffre Dec 1, 2020

Choose a reason for hiding this comment

jumaffre Dec 1, 2020

Choose a reason for hiding this comment

ghost commented Dec 1, 2020 • edited by ghost Loading

jumaffre commented Dec 2, 2020

jumaffre Dec 3, 2020

Choose a reason for hiding this comment

jumaffre Dec 3, 2020

Choose a reason for hiding this comment

jumaffre Dec 3, 2020

Choose a reason for hiding this comment

jumaffre commented Dec 1, 2020 •

edited

Loading

ghost commented Dec 1, 2020 •

edited by ghost

Loading