Feature Support: cluster gc #739

0zAND1z · 2019-04-04T09:53:05Z

This pull request is for a feature request #628

Tasks to be completed before merge:

Implement & review the API client for /repo/gc
Implement & review the RPC Call that needs to be executed in all the cluster nodes
Write tests

This change is

- Ignore the VS Code settings

kishansagathiya

There are a lot of pieces missing in this.
Use #628 (comment) as a checklist
This feature would touch many parts of the codebase so take your time to figure out how they are connected.

RepoGC has to be implemented in IPFSConnector
REST API for RepoGC https://github.com/ipfs/ipfs-cluster/blob/master/api/rest/restapi.go

How I would go about doing this would be

start with getting the rpc api, then rest api then the client+cli

kishansagathiya · 2019-04-05T01:41:17Z

ipfscluster.go

@@ -68,6 +68,7 @@ type API interface {
 type IPFSConnector interface {
 	Component
 	ID(context.Context) (*api.IPFSID, error)
+	GC(context.Context) (*api.IPFSRepoGc, error)


Make it RepoGC and move it after RepoStat

We have to implement this as well

Renamed & Moved. The implementation skeleton has been added at cluster.go.

kishansagathiya · 2019-04-05T01:43:10Z

cluster.go

@@ -605,6 +605,11 @@ func (c *Cluster) ID(ctx context.Context) *api.ID {
 	}
 }

+// GC performs garbage collection in the cluster
+func (c *Cluster) GC(ctx context.Context) *api.IPFSRepoGc {
+	// TODO: Need clarity on how to return the response of c.ipfs.GC(ctx)


Make IPFSRepoGC rpc calls to all peers. See how that's done in other cluster methods.

.gitignore

cluster.go

ipfscluster.go

rpc_api.go

cluster.go

Co-Authored-By: 0zAND1z <11145839+0zAND1z@users.noreply.github.com>

- renaming Co-Authored-By: 0zAND1z <11145839+0zAND1z@users.noreply.github.com>

0zAND1z · 2019-04-04T09:54:00Z

api/ipfsproxy/ipfsproxy.go

+// repoGcHandler launches the garbage collection sequence in the cluster.
+// Based on https://docs.ipfs.io/reference/api/http/#api-v0-repo-gc
+func (proxy *Server) repoGcHandler(w http.ResponseWriter, r *http.Request) {
+	//TODO: Need help/direction on finishing the handler


@kishansagathiya , who can help me here? Can you please tag them for further follow-up? Thanks.

@lanzafame , any suggestions here? Thanks.

I think I already mentioned in #628 (comment)

You will have to create a cluster method(say RepoGC) that would make rpc calls for RepoGC(say IPFSRepoGC) to each peer. You will need to write an rpc api that would call the cluster method described above(RepoGC).
You handlers(in restapi.go and ipfsproxy,go) would make RepoGC(the cluster one, not IPFSRepoGC) rpc call to itself.

Thanks Kishan, working on the changes.

had a small confusion here - should I be use .Call() or .CallContext() ?

I've just committed the code using the .CallContext(). But I'm looking for some inputs here on completing the code.

CallContext

0zAND1z · 2019-04-04T09:54:43Z

cmd/ipfs-cluster-ctl/main.go

@@ -252,6 +252,21 @@ cluster peers.
 						return nil
 					},
 				},
+				{
+					Name:  "gc",


Is it okay to have gc as a sub command ?

My opinion is to create a new command ipfs-cluster-ctl repo gc, but this comes much later (checklist #628 (comment))

@kishansagathiya I disagree that we should have another command just for gc. And I think makes more sense to be under the peers command as this will be broadcast to all peers.

I lean towards ipfs-cluster-ctl peers gc too.

I think of peers subcommand as peerset management (ls, rm). This doesn't get us info or change anything in the peerset.

I think makes more sense to be under the peers command as this will be broadcast to all peers

Out of existing commands peers is surely the most appropriate. Many of our commands deal with all peers. With that logic many commands can fit into peers. But if I keep this narrow view of peers, it doesn't fit as much as ls or rm. It's not intuitive that peers would have gc, especially for those users who have used ipfs. But when they see repo gc, they would know what it would do.

First I thought of ipfs-cluster-ctl gc, but I thought with repo gc, it would leave room for other repo commands to be added.

Also, this feature is different than cluster's primary use(pinset management). With this we would be adding one more resource to manage. Somehow I am inclined to call that resource repo than peers.

At this point it doesn't make sense to add a repo command for one subcommand and it shouldn't be a top level command. It does fit with the model of managing peers and is associated ipfs node

Closing this thread for now...

So @kishansagathiya does have a point. After all, it's not the cluster peers that are being gc'ed but the ipfs ones (reason why I don't like repo gc either). We could do ipfs-cluster-ctl ipfs gc, where the ipfs subcommand runs commands on the ipfs daemons. Additional commands for the future ipfs-cluster-ctl ipfs connect (swarm connect) or... (haven't thought of more).

.gitignore

0zAND1z · 2019-04-05T03:43:41Z

ipfscluster.go

@@ -68,6 +68,7 @@ type API interface {
 type IPFSConnector interface {
 	Component
 	ID(context.Context) (*api.IPFSID, error)
+	GC(context.Context) (*api.IPFSRepoGc, error)


Renamed & Moved. The implementation skeleton has been added at cluster.go.

ipfscluster.go

0zAND1z · 2019-04-20T11:50:59Z

api/ipfsproxy/ipfsproxy.go

+		"",
+		"Cluster",
+		"IPFSRepoGc",
+		&peers,


I just used this from repo stat, not sure if this is the right way to do it. Can somebody confirm if this is alright?

0zAND1z · 2019-04-20T11:52:00Z

cmd/ipfs-cluster-ctl/main.go

@@ -252,6 +252,21 @@ cluster peers.
 						return nil
 					},
 				},
+				{
+					Name:  "gc",


Closing this thread for now...

ipfscluster.go

hsanjuan · 2019-04-23T08:27:25Z

cluster.go

+	ctx = trace.NewContext(c.ctx, span)
+
+	// ignore error since it is included in response object
+	ipfsGC, err := c.ipfs.RepoGC(ctx)


Here, instead of calling the local IPFS RepoGC, you will need to make a broadcast request to every peer. There are examples in this file (anything with rpc.MultiCall). Otherwise leave a comment and I or someone else will fix it when the rest is ready.

kishansagathiya · 2019-05-14T18:57:55Z

I will finish this PR with #777

@0zAND1z
I have squashed your work in a commit and added to that PR
Thanks a lot for this

0zAND1z and others added 3 commits April 4, 2019 15:00

- Ignore the VS Code settings

e9f462f

Merge pull request #1 from 0zAND1z/master

bb2d700

- Ignore the VS Code settings

- cluster-gc code skeleton

4f885f2

hsanjuan self-assigned this Apr 4, 2019

kishansagathiya suggested changes Apr 5, 2019

View reviewed changes

0zAND1z added 4 commits April 5, 2019 09:20

- rename & move the interface definiton for GC

40e9dc6

- RepoGC logic

ce62858

-GC() renamed to RepoGC()

87aff0c

- comment syntax fixed

0557598

lanzafame suggested changes Apr 8, 2019

View reviewed changes

lanzafame and others added 4 commits April 18, 2019 10:06

Update cluster.go

587da84

Co-Authored-By: 0zAND1z <11145839+0zAND1z@users.noreply.github.com>

Update ipfscluster.go

b533d56

Co-Authored-By: 0zAND1z <11145839+0zAND1z@users.noreply.github.com>

Update cluster.go

4674125

Co-Authored-By: 0zAND1z <11145839+0zAND1z@users.noreply.github.com>

Apply suggestions from code review

f68a198

- renaming Co-Authored-By: 0zAND1z <11145839+0zAND1z@users.noreply.github.com>

0zAND1z commented Apr 18, 2019

View reviewed changes

0zAND1z added 3 commits April 18, 2019 10:18

- removed .vscode

a72ee63

- revised the peers gc usage message

3410dc9

- updated the rpc api & the proto

b45fd4d

0zAND1z commented Apr 20, 2019

View reviewed changes

- codeclimate issues fixed

a4d924c

hsanjuan reviewed Apr 23, 2019

View reviewed changes

kishansagathiya mentioned this pull request May 14, 2019

Add ability to run Garbage Collector(ipfs repo gc) on all peers #777

Closed

kishansagathiya closed this May 14, 2019

kishansagathiya mentioned this pull request Oct 24, 2019

Run Garbage Collector(ipfs repo gc) on all peers #945

Merged

hsanjuan mentioned this pull request Nov 7, 2019

Release 0.12.0 #924

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature Support: cluster gc #739

Feature Support: cluster gc #739

0zAND1z commented Apr 4, 2019 •

edited

kishansagathiya left a comment

kishansagathiya Apr 5, 2019

kishansagathiya Apr 5, 2019

0zAND1z Apr 5, 2019

kishansagathiya Apr 5, 2019

0zAND1z Apr 4, 2019

0zAND1z Apr 18, 2019

kishansagathiya Apr 18, 2019

0zAND1z Apr 18, 2019

0zAND1z Apr 20, 2019

hsanjuan Apr 23, 2019

0zAND1z Apr 4, 2019

kishansagathiya Apr 18, 2019 •

edited

lanzafame Apr 18, 2019

hsanjuan Apr 18, 2019

kishansagathiya Apr 18, 2019 •

edited

kishansagathiya Apr 18, 2019

lanzafame Apr 18, 2019

0zAND1z Apr 20, 2019

hsanjuan Apr 24, 2019

0zAND1z Apr 5, 2019

0zAND1z Apr 20, 2019

0zAND1z Apr 20, 2019

hsanjuan Apr 23, 2019

kishansagathiya commented May 14, 2019

Feature Support: cluster gc #739

Feature Support: cluster gc #739

Conversation

0zAND1z commented Apr 4, 2019 • edited

kishansagathiya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kishansagathiya Apr 18, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kishansagathiya Apr 18, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kishansagathiya commented May 14, 2019

0zAND1z commented Apr 4, 2019 •

edited

kishansagathiya Apr 18, 2019 •

edited

kishansagathiya Apr 18, 2019 •

edited