feat: add deduplication ratio to 'ipfs dag stat' by arthurgavazza · Pull Request #9787 · ipfs/kubo

arthurgavazza · 2023-04-01T14:51:06Z

This adds deduplication information on a list of Cids for the ipfs dag stat command,as requested in #9762. ex:
running ipfs dag stat QmcQs9AFNh4mHKqFLJ2Zo7WPSottWi9YDdEoqdk3jv5RyE QmSg4hRo947Lkp9qKn3KJhjjCFGNmtw8VLoXhUCC9mLRgU QmSg4hRo947Lkp9qKn3KJhjjCFGNmtw8VLoXhUCC9mLRgU writes the following output to stdout:

CID                                           	Blocks         	Size
QmcQs9AFNh4mHKqFLJ2Zo7WPSottWi9YDdEoqdk3jv5RyE	2500000        	27
QmSg4hRo947Lkp9qKn3KJhjjCFGNmtw8VLoXhUCC9mLRgU	2500000        	27
QmSg4hRo947Lkp9qKn3KJhjjCFGNmtw8VLoXhUCC9mLRgU	2500000        	27

Summary

Total Size: 54
Unique Blocks: 2
Shared Size: 27
Ratio: 1.500000

and with the --enc=json option:

{
    "UniqueBlocks": 2,
    "TotalSize": 54,
    "SharedSize": 27,
    "Ratio": 1.5,
    "DagStats": [
        {
            "Cid": "QmcQs9AFNh4mHKqFLJ2Zo7WPSottWi9YDdEoqdk3jv5RyE",
            "Size": 27,
            "NumBlocks": 1
        },
        {
            "Cid": "QmSg4hRo947Lkp9qKn3KJhjjCFGNmtw8VLoXhUCC9mLRgU",
            "Size": 27,
            "NumBlocks": 1
        },
        {
            "Cid": "QmSg4hRo947Lkp9qKn3KJhjjCFGNmtw8VLoXhUCC9mLRgU",
            "Size": 27,
            "NumBlocks": 1
        }
    ]
}

Fixes #9762

arthurgavazza · 2023-04-01T14:54:32Z

@Jorropo this is just a draft, I'm still working on it, I'll add the tests and improve some things before starting with the bonus points. Can you take a look at this, just to see if I'm in the right direction? I don't know if that's what you wanted for the json and text encoders.

arthurgavazza · 2023-04-17T11:47:12Z

@Jorropo can you review this draft? I'm kind of blocked on this feature, I just need to know if I'm on the right direction and if this satisfies the issue basic requirements so I can continue working on it and make the improvements you mentioned.

BigLep · 2023-04-19T03:36:30Z

Just wanted to say thanks @arthurgavazza for your contribution. The maintainers have been busy with https://2023.ipfs-thing.io/ but will be back into more normal flow next week.

arthurgavazza · 2023-04-19T22:40:46Z

@BigLep thanks for letting me know!

Jorropo

Thx a lot <3 this is good.
I have a few comments.

core/commands/dag/dag.go

Jorropo · 2023-05-02T14:19:14Z

core/commands/dag/dag.go

+	if s.TotalSize > 0 {
+		s.Ratio = float32(s.redundantSize) / float32(s.TotalSize)
+	}


I would remove the if check and let s.Ratio being NaN and implement custom marshal which omits Ratio if it is NaN. But that fine, probably.

core/commands/dag/dag.go

core/commands/dag/stat.go

core/commands/dag/dag.go

arthurgavazza · 2023-05-02T14:52:04Z

@Jorropo thx for the feedback! I'll work on this ASAP

Co-authored-by: Jorropo <[email protected]>

Jorropo · 2023-05-03T00:33:18Z

For ci fix please run:

find -type f -name "go.mod" | while read f; do cd $(dirname "${f}"); go mod tidy; cd -; done
go fmt ./...

BigLep · 2023-05-11T06:04:21Z

@arthurgavazza: are you to fix CI? It would be great to include in the next release.

Jorropo · 2023-05-11T06:09:36Z

I think some fixes after the review havn't been pushed yet to. 🙂

arthurgavazza · 2023-05-11T17:11:30Z

@BigLep @Jorropo I've been busy with work this week. I still have to finish some fixes, I'm gonna try to do this until tomorrow night if that's ok

Jorropo · 2023-05-11T17:24:28Z

@arthurgavazza you don't have any deadline, feel free to work on this when you have time. It will get in the release cycle whenever that happens.
Thx for your time this is appreciated.

Jorropo · 2023-05-16T07:09:36Z

test/cli/dag_test.go

+		assert.Nil(t, err)
+		stat := node.RunIPFS("dag", "stat", "--progress=false", node1Cid, node2Cid)
+		str := stat.Stdout.String()
+		expected := "\nCID                                           \tBlocks         \tSize\nbafyreibmdfd7c5db4kls4ty57zljfhqv36gi43l6txl44pi423wwmeskwy\t2              \t53\nbafyreie3njilzdi4ixumru4nzgecsnjtu7fzfcwhg7e6s4s5i7cnbslvn4\t2              \t53\n\nSummary\nTotal Size: 145\nUnique Blocks: 3\nShared Size: 53\nRatio: 1.365517\n\n\n"


Please use the ` syntax, this is very hard to me.

sorry for that, I'm gonna make this more readable

@Jorropo I'm having a hard time comparing the text output with a string literal in this test, I added a test using the json encoded output so that I can validate the deduplication behaviour. Is there any other way that I could test the text output?

@arthurgavazza json is good.

For the text one: the point of golden tests is that the code of the test is very dumb.
You create a dataset ahead of time that show all the behaviour you want (here it would be a .car fixture) and then you == or diff the output with some hardcoded values you create at the same time you make your fixture.

test/cli/dag_test.go

Co-authored-by: Jorropo <[email protected]>

arthurgavazza · 2023-06-04T19:42:16Z

@Jorropo sorry for taking too long to make the changes, I was very busy this last few weeks. I added a new test, comparing the text output with a fixture containing the expected results. Can you please take a look at the latest changes?

core/commands/dag/dag.go

hacdias

Very happy that you also moved the old relevant tests from .sh to .go 😃

Co-authored-by: Henrique Dias <[email protected]>

Jorropo

Thx <3

test/cli/dag_test.go

Jorropo · 2023-06-06T13:17:50Z

Thx ❤️🎉

arthurgavazza · 2023-06-06T13:21:49Z

@Jorropo thx for all the support! ❤️ I'm really glad I could help. About the improvements you mentioned on the issue. Can I open a new issue with them and start working on it?

Jorropo · 2023-06-06T14:21:53Z

@arthurgavazza which improvements ?

arthurgavazza · 2023-06-06T14:25:32Z

@Jorropo
Bonus points (you can go for it if you want to have fun but I don't care about it for now):

You execute the multiple traversals in parallel of one an other. That means all iterations of the loop starts a goroutine and you wg.Wait() the final result before printing it.
You add computation of internal compression ratios too (that means CIDs that share blocks with themselves multiple times).
You optimize the code by caching results of the traversal at any point to only traverse blocks once, even if they are duplicated across the different CIDs (hard).
You compute a result in a single pass (medium if done alone, very hard if combined with 1 and 3).
You output rich progress information similar to the final output (instead of just some total amount blocks).

Jorropo · 2023-06-06T14:53:41Z

@arthurgavazza I don't care that much about thoses, 1, 3 and 4 are just performance related (and what you have now is ok). 2 and 5 are very minor quality of life.

Feel free to do it if that is fun for you, but if you want to make something that would change Kubo's usability I think there will be better use of your time than over optimizing an underused endpoint. 🙂

arthurgavazza added 6 commits March 31, 2023 18:23

checkpoint1

f1604da

feat: achieve correct minimal output

ac3e6b0

feat: start adding json encoding

ce81fd0

fix: fmt

6dffb58

feat: add custom json encoder and format text encoder

946fdd6

fix: remove test json files

63d271b

Jorropo self-requested a review April 4, 2023 02:40

Jorropo requested changes May 2, 2023

View reviewed changes

core/commands/dag/dag.go Outdated Show resolved Hide resolved

core/commands/dag/dag.go Outdated Show resolved Hide resolved

arthurgavazza and others added 5 commits May 2, 2023 21:06

Update core/commands/dag/dag.go

fbc1c9c

Co-authored-by: Jorropo <[email protected]>

fix: check error on cdv write

eac10df

Co-authored-by: Jorropo <[email protected]>

fix: tsv title format

27547d3

Co-authored-by: Jorropo <[email protected]>

fix: print line with Fprintln

7f38e7e

Co-authored-by: Jorropo <[email protected]>

fix: use boxo traverse

76e5f20

Co-authored-by: Jorropo <[email protected]>

arthurgavazza added 2 commits May 2, 2023 21:37

refactor: calculate summary with only one method

b48dcb0

refactor: make simplifications

9bbef7f

BigLep marked this pull request as ready for review May 11, 2023 06:04

BigLep requested a review from a team as a code owner May 11, 2023 06:04

arthurgavazza added 2 commits May 12, 2023 13:24

fix: add support to progressive information

620f73c

fix: fix ci

9146d95

Jorropo requested changes May 16, 2023

View reviewed changes

Update test/cli/dag_test.go

8d79bcb

Co-authored-by: Jorropo <[email protected]>

BigLep mentioned this pull request May 18, 2023

Kubo/Boxo: pre PR signal that a contributor needs to run "go mod tidy" ipfs/boxo#309

Closed

arthurgavazza and others added 4 commits May 21, 2023 13:17

fix: deduplication calculations and tests

f1840ca

fix: lint

864e91b

fix: CI

b689c10

Merge branch 'master' into feat/add-deduplication-ratio-stat

220856f

Jorropo mentioned this pull request May 30, 2023

feat: remove ipld legacy and format globals in ipld/merkledag ipfs/boxo#322

Merged

arthurgavazza and others added 2 commits June 4, 2023 16:20

test: add text output tests

f2051db

Merge branch 'master' into feat/add-deduplication-ratio-stat

0df029e

hacdias requested a review from Jorropo June 5, 2023 09:42

hacdias reviewed Jun 5, 2023

View reviewed changes

core/commands/dag/dag.go Outdated Show resolved Hide resolved

hacdias reviewed Jun 5, 2023

View reviewed changes

hacdias changed the title ~~Feat/add deduplication ratio stat~~ feat: add deduplication ratio to 'ipfs dag stat' Jun 5, 2023

Update core/commands/dag/dag.go

08f837c

Co-authored-by: Henrique Dias <[email protected]>

Jorropo requested changes Jun 5, 2023

View reviewed changes

test/cli/dag_test.go Outdated Show resolved Hide resolved

Jorropo added 2 commits June 5, 2023 12:47

Update test/cli/dag_test.go

538f5bb

FIXUP

336a98c

hacdias requested a review from Jorropo June 6, 2023 13:16

Jorropo approved these changes Jun 6, 2023

View reviewed changes

Jorropo merged commit 726eabe into ipfs:master Jun 6, 2023

arthurgavazza deleted the feat/add-deduplication-ratio-stat branch June 6, 2023 13:21

Jorropo mentioned this pull request Jun 6, 2023

docs: changelog for dag stat #9919

Merged

hacdias mentioned this pull request Jun 27, 2023

Revert "feat: add dag stat method (#297)" ipfs/go-ipfs-api#298

Merged

Uh oh!

Conversation

arthurgavazza commented Apr 1, 2023 • edited by Jorropo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arthurgavazza commented Apr 1, 2023

Uh oh!

arthurgavazza commented Apr 17, 2023

Uh oh!

BigLep commented Apr 19, 2023

Uh oh!

arthurgavazza commented Apr 19, 2023

Uh oh!

Jorropo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jorropo May 2, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arthurgavazza commented May 2, 2023

Uh oh!

Jorropo commented May 3, 2023

Uh oh!

BigLep commented May 11, 2023

Uh oh!

Jorropo commented May 11, 2023

Uh oh!

arthurgavazza commented May 11, 2023

Uh oh!

Jorropo commented May 11, 2023

Uh oh!

Jorropo May 16, 2023

Choose a reason for hiding this comment

Uh oh!

arthurgavazza May 16, 2023

Choose a reason for hiding this comment

Uh oh!

arthurgavazza May 21, 2023

Choose a reason for hiding this comment

Uh oh!

Jorropo May 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arthurgavazza commented Jun 4, 2023

Uh oh!

Uh oh!

hacdias left a comment

Choose a reason for hiding this comment

Uh oh!

Jorropo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jorropo commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arthurgavazza commented Jun 6, 2023

Uh oh!

Jorropo commented Jun 6, 2023

Uh oh!

arthurgavazza commented Jun 6, 2023 • edited by Jorropo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jorropo commented Jun 6, 2023

Uh oh!

Reviewers

Assignees

Labels

arthurgavazza commented Apr 1, 2023 •

edited by Jorropo

Loading

Jorropo commented Jun 6, 2023 •

edited

Loading

arthurgavazza commented Jun 6, 2023 •

edited by Jorropo

Loading