Radish alpha
h
heartwood
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
Patches › 7e8e103b
radicle: improve quorum copy
Merged fintohaps opened 1 year ago

The no quorum found error can be opaque and lead to a lot of confusion. It is better to give more information for this error so that it leads to easier debugging, since it can be quite a common warning/error.

Instead of a single error, there are two cases provided: NoCandidates and Diverging, to capture the two different variants of errors that can occur.

Display implementations are provided for both to provide more information.

Activity Changes +183 -51
4 files changed +183 -51 159d3fce → 91914d93
modified radicle-cli/examples/git/git-push-converge.md
@@ -59,13 +59,13 @@ found` error is showing up:
$ rad remote add did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --name bob

✓ Follow policy updated for z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (bob)

✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z..
- 
✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. error: no quorum was found
+ 
✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk..

✓ Remote bob added

✓ Remote-tracking branch bob/master created for z6Mkt67…v4N1tRk

$ rad remote add did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z --name eve

✓ Follow policy updated for z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z (eve)
- 
✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z.. error: no quorum was found
- 
✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. error: no quorum was found
+ 
✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z..
+ 
✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk..

✓ Remote eve added

✓ Remote-tracking branch eve/master created for z6Mkux1…nVhib7Z

```
@@ -89,9 +89,10 @@ commit:


``` ~alice (stderr)

$ git push rad -f
- 
warn: no quorum was found for `refs/heads/master`
+ 
warn: could not determine canonical tip for `refs/heads/master`
+ 
warn: no commit found with at least 3 vote(s) (threshold not met)

warn: it is recommended to find a commit to agree upon
- 
✓ Synced with 1 node(s)
+ 
✓ Synced with 2 node(s)

To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi

   d09e634..0f9bd80  master -> master

```
@@ -99,8 +100,8 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE
``` ~bob

$ rad remote add did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi --name alice

✓ Follow policy updated for z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (alice)
- 
✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z.. error: no quorum was found
- 
✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. error: no quorum was found
+ 
✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z..
+ 
✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi..

✓ Remote alice added

✓ Remote-tracking branch alice/master created for z6MknSL…StBU8Vi

$ git reset --hard alice/master
modified radicle-node/src/worker/fetch.rs
@@ -11,7 +11,8 @@ use radicle::identity::DocAt;
use radicle::prelude::RepoId;

use radicle::storage::refs::RefsAt;

use radicle::storage::{
- 
    ReadRepository, ReadStorage as _, RefUpdate, RemoteRepository, WriteRepository as _,
+ 
    ReadRepository, ReadStorage as _, RefUpdate, RemoteRepository, RepositoryError,
+ 
    WriteRepository as _,

};

use radicle::{cob, git, node, Storage};

use radicle_fetch::{Allowed, BlockList, FetchLimit};
@@ -87,6 +88,8 @@ impl Handle {
        remote: PublicKey,

        refs_at: Option<Vec<RefsAt>>,

    ) -> Result<FetchResult, error::Fetch> {
+ 
        use git::canonical::QuorumError::{Diverging, NoCandidates};
+ 


        let (result, clone, notifs) = match self {

            Self::Clone { mut handle, tmp } => {

                log::debug!(target: "worker", "{} cloning from {remote}", handle.local());
@@ -135,7 +138,20 @@ impl Handle {
                // points to a repository that is temporary and gets moved by [`mv`].

                let repo = storage.repository(rid)?;

                repo.set_identity_head()?;
- 
                repo.set_head()?;
+ 
                match repo.set_head() {
+ 
                    Ok(head) => {
+ 
                        if head.is_updated() {
+ 
                            log::trace!(target: "worker", "Set HEAD to {}", head.new);
+ 
                        }
+ 
                    }
+ 
                    Err(RepositoryError::Quorum(Diverging(e))) => {
+ 
                        log::warn!(target: "worker", "Fetch could not set HEAD: {e}")
+ 
                    }
+ 
                    Err(RepositoryError::Quorum(NoCandidates(e))) => {
+ 
                        log::warn!(target: "worker", "Fetch could not set HEAD: {e}")
+ 
                    }
+ 
                    Err(e) => return Err(e.into()),
+ 
                }



                // Notifications are only posted for pulls, not clones.

                if let Some(mut store) = notifs {
modified radicle-remote-helper/src/push.rs
@@ -306,8 +306,18 @@ pub fn run(
                                        ));

                                    }

                                }
- 
                                Err(canonical::QuorumError::NoQuorum) => {
- 
                                    warn(format!("no quorum was found for `{canonical_ref}`"));
+ 
                                Err(canonical::QuorumError::Diverging(e)) => {
+ 
                                    warn(format!(
+ 
                                        "could not determine canonical tip for `{canonical_ref}`"
+ 
                                    ));
+ 
                                    warn(e.to_string());
+ 
                                    warn("it is recommended to find a commit to agree upon");
+ 
                                }
+ 
                                Err(canonical::QuorumError::NoCandidates(e)) => {
+ 
                                    warn(format!(
+ 
                                        "could not determine canonical tip for `{canonical_ref}`"
+ 
                                    ));
+ 
                                    warn(e.to_string());

                                    warn("it is recommended to find a commit to agree upon");

                                }

                                Err(e) => return Err(e.into()),
modified radicle/src/git/canonical.rs
@@ -1,4 +1,5 @@
use std::collections::BTreeMap;
+ 
use std::fmt;



use nonempty::NonEmpty;

use raw::Repository;
@@ -27,14 +28,61 @@ pub struct Canonical {
/// Error that can occur when calculation the [`Canonical::quorum`].

#[derive(Debug, Error)]

pub enum QuorumError {
- 
    /// Could not determine a quorum [`Oid`].
- 
    #[error("no quorum was found")]
- 
    NoQuorum,
+ 
    /// Could not determine a quorum [`Oid`], due to diverging tips.
+ 
    #[error("could not determine canonical reference tip, {0}")]
+ 
    Diverging(Diverging),
+ 
    /// Could not determine a base candidate from the given set of delegates.
+ 
    #[error("could not determine canonical reference tip, {0}")]
+ 
    NoCandidates(NoCandidates),

    /// An error occurred from [`git2`].

    #[error(transparent)]

    Git(#[from] git2::Error),

}
+ 
/// No candidates were found for the [`Canonical::quorum`] calculation.
+ 
///
+ 
/// The [`fmt::Display`] is used in [`QuorumError`], to provide information on
+ 
/// the threshold and delegates in the calculation.
+ 
#[derive(Debug)]
+ 
pub struct NoCandidates {
+ 
    threshold: usize,
+ 
}
+
+ 
impl fmt::Display for NoCandidates {
+ 
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ 
        let NoCandidates { threshold } = self;
+ 
        write!(
+ 
            f,
+ 
            "no commit found with at least {threshold} vote(s) (threshold not met)"
+ 
        )
+ 
    }
+ 
}
+
+ 
/// Diverging commits were found during the [`Canonical::quorum`] calculation.
+ 
///
+ 
/// The [`fmt::Display`] is used in [`QuorumError`], to provide information on
+ 
/// the threshold, base commit, and the two diverging commits, in the
+ 
/// calculation.
+ 
#[derive(Debug)]
+ 
pub struct Diverging {
+ 
    threshold: usize,
+ 
    base: Oid,
+ 
    longest: Oid,
+ 
    head: Oid,
+ 
}
+
+ 
impl fmt::Display for Diverging {
+ 
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ 
        let Diverging {
+ 
            threshold,
+ 
            base,
+ 
            longest,
+ 
            head,
+ 
        } = self;
+ 
        write!(f, "found diverging commits {longest} and {head}, with base commit {base} and threshold {threshold}")
+ 
    }
+ 
}
+ 


impl Canonical {

    /// Construct the set of canonical tips of the `Project::default_branch` for

    /// the given `delegates`.
@@ -150,8 +198,9 @@ impl Canonical {
        // Keep commits which pass the threshold.

        candidates.retain(|_, votes| *votes >= threshold);
- 
        // Keep track of the longest identity branch.
- 
        let (mut longest, _) = candidates.pop_first().ok_or(QuorumError::NoQuorum)?;
+ 
        let (mut longest, _) = candidates
+ 
            .pop_first()
+ 
            .ok_or_else(|| QuorumError::NoCandidates(NoCandidates { threshold }))?;



        // Now that all scores are calculated, figure out what is the longest branch

        // that passes the threshold. In case of divergence, return an error.
@@ -185,7 +234,12 @@ impl Canonical {
                //            o (base)

                //            |

                //
- 
                return Err(QuorumError::NoQuorum);
+ 
                return Err(QuorumError::Diverging(Diverging {
+ 
                    threshold,
+ 
                    base: base.into(),
+ 
                    longest,
+ 
                    head: *head,
+ 
                }));

            }

        }

        Ok((*longest).into())
@@ -281,8 +335,8 @@ mod tests {
        assert_eq!(quorum(&[*c1], 1, &repo).unwrap(), c1);

        assert_eq!(quorum(&[*c2], 1, &repo).unwrap(), c2);

        assert_eq!(quorum(&[*c0], 0, &repo).unwrap(), c0);
- 
        assert_matches!(quorum(&[], 0, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*c0], 2, &repo), Err(QuorumError::NoQuorum));
+ 
        assert_matches!(quorum(&[], 0, &repo), Err(QuorumError::NoCandidates(_)));
+ 
        assert_matches!(quorum(&[*c0], 2, &repo), Err(QuorumError::NoCandidates(_)));



        //  C1

        //  |
@@ -308,19 +362,31 @@ mod tests {
        //  C0

        assert_matches!(

            quorum(&[*c1, *c2, *b2], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*c2, *b2], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*b2, *c2], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*c2, *b2], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*b2, *c2], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))

        );
- 
        assert_matches!(quorum(&[*c2, *b2], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*b2, *c2], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*c2, *b2], 2, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*b2, *c2], 2, &repo), Err(QuorumError::NoQuorum));

        assert_eq!(quorum(&[*c1, *c2, *b2], 2, &repo).unwrap(), c1);

        assert_eq!(quorum(&[*c1, *c2, *b2], 3, &repo).unwrap(), c1);

        assert_eq!(quorum(&[*b2, *b2, *c2], 2, &repo).unwrap(), b2);

        assert_eq!(quorum(&[*b2, *c2, *c2], 2, &repo).unwrap(), c2);

        assert_matches!(

            quorum(&[*b2, *b2, *c2, *c2], 2, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );



        // B2 C2 C3
@@ -331,15 +397,15 @@ mod tests {
        assert_eq!(quorum(&[*b2, *c2, *c2], 2, &repo).unwrap(), c2);

        assert_matches!(

            quorum(&[*b2, *c2, *c2], 3, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::NoCandidates(_))

        );

        assert_matches!(

            quorum(&[*b2, *c2, *b2, *c2], 3, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::NoCandidates(_))

        );

        assert_matches!(

            quorum(&[*c3, *b2, *c2, *b2, *c2, *c3], 3, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::NoCandidates(_))

        );



        //  B2 C2
@@ -349,19 +415,19 @@ mod tests {
        //   C0

        assert_matches!(

            quorum(&[*c2, *b2, *a1], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );

        assert_matches!(

            quorum(&[*c2, *b2, *a1], 2, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::NoCandidates(_))

        );

        assert_matches!(

            quorum(&[*c2, *b2, *a1], 3, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::NoCandidates(_))

        );

        assert_matches!(

            quorum(&[*c1, *c2, *b2, *a1], 4, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::NoCandidates(_))

        );

        assert_eq!(quorum(&[*c0, *c1, *c2, *b2, *a1], 2, &repo).unwrap(), c1,);

        assert_eq!(quorum(&[*c0, *c1, *c2, *b2, *a1], 3, &repo).unwrap(), c1,);
@@ -369,23 +435,23 @@ mod tests {
        assert_eq!(quorum(&[*c0, *c1, *c2, *b2, *a1], 4, &repo).unwrap(), c0,);

        assert_matches!(

            quorum(&[*a1, *a1, *c2, *c2, *c1], 2, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );

        assert_matches!(

            quorum(&[*a1, *a1, *c2, *c2, *c1], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );

        assert_matches!(

            quorum(&[*a1, *a1, *c2], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );

        assert_matches!(

            quorum(&[*b2, *b2, *c2, *c2], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );

        assert_matches!(

            quorum(&[*b2, *b2, *c2, *c2, *a1], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))

        );



        //    M2  M1
@@ -396,15 +462,30 @@ mod tests {
        //       \|

        //       C0

        assert_eq!(quorum(&[*m1], 1, &repo).unwrap(), m1);
- 
        assert_matches!(quorum(&[*m1, *m2], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m2, *m1], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m1, *m2], 2, &repo), Err(QuorumError::NoQuorum));
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *m2], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m2, *m1], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *m2], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))
+ 
        );

        assert_matches!(

            quorum(&[*m1, *m2, *c2], 1, &repo),
- 
            Err(QuorumError::NoQuorum)
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *a1], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *a1], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))

        );
- 
        assert_matches!(quorum(&[*m1, *a1], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m1, *a1], 2, &repo), Err(QuorumError::NoQuorum));

        assert_eq!(quorum(&[*m1, *m2, *b2, *c1], 4, &repo).unwrap(), c1);

        assert_eq!(quorum(&[*m1, *m1, *b2], 2, &repo).unwrap(), m1);

        assert_eq!(quorum(&[*m1, *m1, *c2], 2, &repo).unwrap(), m1);
@@ -440,8 +521,14 @@ mod tests {
        //   C1 C2 C3

        //     \| /

        //      C0
- 
        assert_matches!(quorum(&[*m1, *m2], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m1, *m2], 2, &repo), Err(QuorumError::NoQuorum));
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *m2], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *m2], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))
+ 
        );



        let m3 = fixtures::commit("M3", &[*c2, *c1], &repo);
@@ -450,11 +537,29 @@ mod tests {
        //   C1 C2 C3

        //     \| /

        //      C0
- 
        assert_matches!(quorum(&[*m1, *m3], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m1, *m3], 2, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m3, *m1], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m3, *m1], 2, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m3, *m2], 1, &repo), Err(QuorumError::NoQuorum));
- 
        assert_matches!(quorum(&[*m3, *m2], 2, &repo), Err(QuorumError::NoQuorum));
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *m3], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m1, *m3], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m3, *m1], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m3, *m1], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m3, *m2], 1, &repo),
+ 
            Err(QuorumError::Diverging(_))
+ 
        );
+ 
        assert_matches!(
+ 
            quorum(&[*m3, *m2], 2, &repo),
+ 
            Err(QuorumError::NoCandidates(_))
+ 
        );

    }

}