feat: switch to raw multihashes for blocks #6816

Stebalien · 2020-01-07T17:44:05Z

Part of: #6815

hsanjuan · 2020-02-07T12:05:16Z

@Stebalien other than the right import versions here, this should be good.

gc/gc.go

hsanjuan · 2020-02-28T14:49:58Z

@Stebalien some sharness tests are failing because some outputs changed from CidV0 to CidV1 (i.e. repo gc) and text output is defaulting to base32 then.

Shall I patch at the core/commands level (i.e. we can convert to CidV0 in the places we do .Emit), or update sharness tests? This seems to affect tests relying on refs local and repo gc outputs mostly.

I think things get messy with sharness. If I'm getting it right, blockstore.AllKeysChan is now going to return a list of CidV1.Raw (this affects refs local) regardless of what was the original CID type. But ipfs add will return a CidV1-DagPB.

So a sharness test adding a hash with CidV1 and then checking that refs local shows that hash will fail. Same for some things checking GC output. May also start triggering false positives. Might fix a bunch of those by adding an argument to refs local that allows to check for a single CID (rather than listing and grepping).

hsanjuan · 2020-03-09T15:02:21Z

I'm going to work on fixing sharness tests directly

hsanjuan · 2020-03-09T20:15:53Z

Yai all green :) @Stebalien can you check my last commits?

(btw it seems it fast-forwarded a merge from master or something and did not create a merge commit, making the history a bit dirty).

hsanjuan · 2021-04-21T14:07:58Z

This one seems ok in terms of tests and everything, it just needed some rebase.

hsanjuan · 2021-04-21T14:09:58Z

I will look at it more closely though, particularly the GC part.

Part of: #6815

Not that they will ever happen in the current implementation, but it makes the linter happy and covers our back for future.

This fixes some tests which expect "refs local" and "repo gc" outputs to match the CIDs produced when adding data. These operations are now outputting CIDv1-raw hashes, regardless of the original CIDs used to address those blocks, so some tests fail. The fix is usually: * To use "block stat" to check if a block was correctly gc'ed * To convert the CIDs to multihash (using cid-fmt) and compare those instead

This sharness test provides 2 datastore blocks directly. One of them (corresponding to an "insecure block") is not a CIDv0 therefore it needs to be migrated and stored with its raw-multihash ID. The (raw) CID of the block used in the test remains the same. The other block needs no changes.

Compare multihashes rather than CIDs.

aschmahmann · 2021-12-02T15:14:43Z

@hsanjuan the only thing I clobbered/ignored in the rebase was 163794e (t0081-repo-pinning.sh re-enable the offline daemon), since I figured it might've been intentional that we were running some of the tests with the daemon off and some with it on, but in offline mode.

If so we can just remove the extraneous comments. This issue seems unrelated to the main thrust of the PR, but ended up here as you were cleaning things up (thanks for that 🙏).

gc/gc.go

aschmahmann · 2021-12-02T15:53:23Z

test/sharness/t0080-repo.sh

+  HASH_MH=`cid-fmt -b base32 "%M" "$HASH"` &&
+  HARDCODED_HASH_MH=`cid-fmt -b base32 "%M" "QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y"` &&
+  EMPTY_DIR_MH=`cid-fmt -b base32 "%M" "$EMPTY_DIR"` &&
+  HASH_WELCOME_DOCS_MH=`cid-fmt -b base32 "%M" "$HASH_WELCOME_DOCS"` &&
+  ipfs refs local | cid-fmt -b base32 --filter "%M" >actual8 &&


Do we need cid-fmt here, or can we use ipfs cid format? Generally, asking here and through the PR.

I don't know, this was done before ipfs cid format was there. Worth fixing in a different PR accross all tests?

test/sharness/t0087-repo-robust-gc.sh

aschmahmann · 2021-12-09T17:54:25Z

test/sharness/t0087-repo-robust-gc.sh

+    grep -q "removed $(to_raw_cid $LEAF3)" repo_gc_out &&
+    grep -q "removed $(to_raw_cid $LEAF4)" repo_gc_out &&


These don't seem necessary since we're already doing raw leaves, but it can't hurt

aschmahmann · 2021-12-09T17:54:39Z

test/sharness/t0087-repo-robust-gc.sh

@@ -85,7 +89,7 @@ test_gc_robust_part1() {

  test_expect_success "'ipfs repo gc' should be ok now" '
    ipfs repo gc | tee repo_gc_out
-    grep -q "removed $LEAF2" repo_gc_out
+    grep -q "removed $(to_raw_cid $LEAF2)" repo_gc_out


This doesn't seem necessary since we're already doing raw leaves, but it can't hurt

aschmahmann · 2021-12-09T17:55:42Z

go.mod

 	github.com/ipfs/go-fs-lock v0.0.7
 	github.com/ipfs/go-graphsync v0.11.0
-	github.com/ipfs/go-ipfs-blockstore v0.2.1
+	github.com/ipfs/go-ipfs-blockstore v1.1.2


TODO: Update dist CID to point at latest migration

aschmahmann · 2021-12-13T19:51:35Z

repo/fsrepo/fsrepo.go

@@ -36,7 +36,7 @@ const LockFile = "repo.lock"
 var log = logging.Logger("fsrepo")

 // version number that we are currently expecting to see
-var RepoVersion = 11
+var RepoVersion = 12


Note: bumped repo version

aschmahmann · 2021-12-13T19:53:37Z

repo/fsrepo/migrations/fetcher.go

@@ -10,7 +10,7 @@ import (

 const (
 	// Current distribution to fetch migrations from
-	CurrentIpfsDist = "/ipfs/QmP7tLxzhLU1KauTRX3jkVkF93pCv4skcceyUYMhf4AKJR" // fs-repo-migrations v2.0.2
+	CurrentIpfsDist = "/ipfs/QmPweMoUxWFt1MSpYwLWsHB1GBcyYYDKPnANdERMY4U6hK" // fs-repo-11-to-12 v1.0.1


Note: Bumped to latest version of dist.ipfs.io

github-actions bot requested a review from hsanjuan January 7, 2020 17:45

Stebalien mentioned this pull request Jan 7, 2020

Switch to raw multihashes in datastores #6815

Closed

12 tasks

Stebalien force-pushed the feat/mh-backed-datastore branch 3 times, most recently from 5535749 to df993a3 Compare January 8, 2020 19:49

hsanjuan self-assigned this Jan 27, 2020

hsanjuan force-pushed the feat/mh-backed-datastore branch from df993a3 to 19bdde9 Compare February 3, 2020 13:24

hsanjuan mentioned this pull request Feb 6, 2020

Migration 11-12: migrate CIDsv1 to raw multihashes ipfs/fs-repo-migrations#95

Merged

Stebalien commented Feb 7, 2020

View reviewed changes

gc/gc.go Outdated Show resolved Hide resolved

hsanjuan force-pushed the feat/mh-backed-datastore branch from aa3757e to f0f8f89 Compare February 28, 2020 13:18

hsanjuan force-pushed the feat/mh-backed-datastore branch from 2e32814 to 24f9109 Compare April 21, 2021 12:35

hsanjuan requested a review from aschmahmann April 21, 2021 14:07

Stebalien and others added 7 commits December 2, 2021 09:41

feat: switch to raw multihashes for blocks

7f27dbf

Part of: #6815

gc: remove leftover tracing

fc56055

gc: handle errs in toRawCids()

8d2d918

Not that they will ever happen in the current implementation, but it makes the linter happy and covers our back for future.

Sharness: fix t0084-repo-read-rehash for raw multihashes

953f4b6

Compare multihashes rather than CIDs.

Bump go-filestore and go-blockstore dependencies

b55a943

aschmahmann force-pushed the feat/mh-backed-datastore branch from d219ad7 to b55a943 Compare December 2, 2021 15:06

aschmahmann reviewed Dec 2, 2021

View reviewed changes

fix: reenable some gc tests

50abfad

test: fix CIDs in repo gc sharness tests

cdb76ad

hsanjuan approved these changes Dec 9, 2021

View reviewed changes

chore: update go-ipfs-blockstore

9913183

aschmahmann reviewed Dec 9, 2021

View reviewed changes

This was referenced Dec 9, 2021

WIP: Use new blockstore that stores Multihashes and not CIDs. #5510

Closed

Naming blocks in the datastore ipfs/specs#242

Open

feat: bump repo version to 12 and update CID of the distributions

7fe5650

aschmahmann reviewed Dec 13, 2021

View reviewed changes

aschmahmann merged commit 1a040b0 into master Dec 13, 2021

aschmahmann deleted the feat/mh-backed-datastore branch December 13, 2021 20:37

aschmahmann mentioned this pull request Dec 13, 2021

Release v0.12 #8344

Closed

59 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: switch to raw multihashes for blocks #6816

feat: switch to raw multihashes for blocks #6816

Stebalien commented Jan 7, 2020

hsanjuan commented Feb 7, 2020

hsanjuan commented Feb 28, 2020 •

edited

Loading

hsanjuan commented Mar 9, 2020

hsanjuan commented Mar 9, 2020

hsanjuan commented Apr 21, 2021

hsanjuan commented Apr 21, 2021

aschmahmann commented Dec 2, 2021

aschmahmann Dec 2, 2021

hsanjuan Dec 7, 2021

aschmahmann Dec 9, 2021

aschmahmann Dec 9, 2021

aschmahmann Dec 9, 2021

aschmahmann Dec 13, 2021

aschmahmann Dec 13, 2021

		grep -q "removed $(to_raw_cid $LEAF3)" repo_gc_out &&
		grep -q "removed $(to_raw_cid $LEAF4)" repo_gc_out &&

feat: switch to raw multihashes for blocks #6816

feat: switch to raw multihashes for blocks #6816

Conversation

Stebalien commented Jan 7, 2020

hsanjuan commented Feb 7, 2020

hsanjuan commented Feb 28, 2020 • edited Loading

hsanjuan commented Mar 9, 2020

hsanjuan commented Mar 9, 2020

hsanjuan commented Apr 21, 2021

hsanjuan commented Apr 21, 2021

aschmahmann commented Dec 2, 2021

aschmahmann Dec 2, 2021

Choose a reason for hiding this comment

hsanjuan Dec 7, 2021

Choose a reason for hiding this comment

aschmahmann Dec 9, 2021

Choose a reason for hiding this comment

aschmahmann Dec 9, 2021

Choose a reason for hiding this comment

aschmahmann Dec 9, 2021

Choose a reason for hiding this comment

aschmahmann Dec 13, 2021

Choose a reason for hiding this comment

aschmahmann Dec 13, 2021

Choose a reason for hiding this comment

hsanjuan commented Feb 28, 2020 •

edited

Loading