ffs: enable auto-repair & update to new interopnet & ipfs pinset cache #272

jsign · 2020-04-22T22:45:30Z

Fixes Enable a Repair config #18
Fixes Cache Pinset of IPFS node #228
Fixes ffs: update to new interopnet #314 which updates to to new interop, which solved a bug we found which ended up making 3 or 4 tests to be skipped. Updating to the new interop involved some work on Powergate and:
Allowed all tests in ffs/integrationtest to run in parallel, and enabled 4 parallel runs in CI. I had to find the right tune between the devnets speeds and the parallelization count. Pushing too much start making tests flaky since CPU are %100. This decreased that package test runs from ~400s to ~100s.
I had to disable some index tests since in the new interop there're some problems. Those tests where quite meaningless already anyway (devnet is quite artificial for miners metadata), but I'll re-enable when gets fixed.
The good part is that I re-enabled most of the disabled ffs/integrationtest that were disabled because of the double-deal problem that is fixed, that feels good since I consider those more important ones.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign · 2020-04-24T12:00:36Z

deals/deals.go

-			EpochPrice:     types.NewInt(c.EpochPrice),
-			Miner:          maddr,
-			Wallet:         addr,
+			MinBlocksDuration: dur,


New interopnet API change.

jsign · 2020-04-24T12:00:53Z

fchost/config.go

+		"/dns4/t01000.miner.interopnet.kittyhawk.wtf/tcp/1347/p2p/12D3KooWNjqGTNZ592wG5DmX7Rbmb76V3NHMdZMqjqPJ1xiax7w6",
+		"/ip4/54.186.82.90/tcp/1347/p2p/12D3KooWGf4ggd3cdZJKZPHkgSe8Lxf4MDsKhVC7npRGXgsPP4fz",


New bootstrap peers of interopnet.

jsign · 2020-04-24T12:02:12Z

ffs/coreipfs/coreipfs.go

@@ -5,6 +5,7 @@ import (
 	"context"


Make a pinset cache. Basically is loaded on first use instead of in New. From that point forward, since PG is the only one touching the pinset of the IPFS node it's safe to assume that cache will be always valid and the API call to the IPFS node to get pinsets can be avoided.

It's an in-memory only thing?

Yep, just a guarded map of cid.Cid.

jsign · 2020-04-24T12:02:47Z

ffs/integrationtest/integration_test.go

@@ -209,7 +219,7 @@ func TestInfo(t *testing.T) {
 		requireCidConfig(t, fapi, cid, nil)
 	}

-	t.Run("WithOneAdd", func(t *testing.T) {
+	t.Run("WithThreeAdd", func(t *testing.T) {


Re-enable the test that made 3 deals of different data with a single miner. 💯

jsign · 2020-04-24T12:03:18Z

ffs/integrationtest/integration_test.go

@@ -323,10 +336,7 @@ func TestRepFactor(t *testing.T) {
 }

 func TestRepFactorIncrease(t *testing.T) {
-	// ToDo: unskip when testnet/3  allows more than one deal
-	// See https://bit.ly/2JxQSQk
-	t.SkipNow()


jsign · 2020-04-24T12:07:28Z

ffs/types.go

+}
+
+// WithRepairable allows to enable/disable auto-repair.
+func (c CidConfig) WithRepairable(enabled bool) CidConfig {


The new option for CidConfig.

jsign · 2020-04-24T12:08:47Z

index/ask/ask.go

@@ -201,12 +201,12 @@ func generateIndex(ctx context.Context, api *apistruct.FullNodeStruct) (*IndexSn
 			defer func() { <-rateLim }()
 			ictx, cancel := context.WithTimeout(ctx, qaTimeout)
 			defer cancel()
-			pid, err := api.StateMinerPeerID(ictx, addr, types.EmptyTSK)


StateMinerPeerID was removed from Lotus API.
Now it seems that you should make a more general API call for miner's info which has the PeerID.
The returned PeerID still seems to have some extra problems when used with QueryAsks (as mentioned in PR description about index-tests).

jsign · 2020-04-24T12:09:16Z

index/miner/onchain.go

-	p := mp.MinerPower.Uint64()
+	p := mp.MinerPower.RawBytePower.Uint64()


Lotus API change.

jsign · 2020-04-24T12:10:44Z

index/slashing/slashing_test.go

+	// Skipped until #235 lands.
+	t.SkipNow()


Is green locally, but is flaky on CI.
There's some pending work to do about the slashing index anyway, so this test doesn't make much sense. In fact, it never made much sense since there isn't a good way to force slashing.

jsign · 2020-04-24T12:11:50Z

tests/ldevnet.go

@@ -21,7 +21,8 @@ func LaunchDevnetDocker(t *testing.T, numMiners int) *dockertest.Resource {
 		panic(fmt.Sprintf("couldn't create ipfs-pool: %s", err))
 	}
 	envNumMiners := fmt.Sprintf("TEXLOTUSDEVNET_NUMMINERS=%d", numMiners)
-	lotusDevnet, err := pool.RunWithOptions(&dockertest.RunOptions{Repository: "textile/lotus-devnet", Tag: "sha-9aab2c6", Env: []string{envNumMiners}, Mounts: []string{"/tmp/powergate:/tmp/powergate"}})
+	speed := "TEXLOTUSDEVNET_SPEED=500"


I made devnets run a bit slower than default. This allow running more in parallel without hurting the CI CPUs.
The default speed is quite fast anyway for a single test run, so in the bottom line this has better performance.

asutula

Great work. Huge step forward for CI and nice how simple it was to add repair!

asutula · 2020-04-24T14:56:06Z

ffs/scheduler/scheduler.go

@@ -207,7 +210,31 @@ func (s *Scheduler) run() {
 	}
 }

-func (s *Scheduler) scanRenewable(ctx context.Context) {
+func (s *Scheduler) execRepairCron(ctx context.Context) {


jsign · 2020-04-24T15:04:40Z

Great work. Huge step forward for CI and nice how simple it was to add repair!

Thanks for the review!

sanderpick

LGTM!

sanderpick · 2020-04-24T15:19:10Z

ffs/coreipfs/coreipfs.go

@@ -5,6 +5,7 @@ import (
 	"context"


sanderpick · 2020-04-24T15:20:18Z

ffs/coreipfs/coreipfs.go

@@ -5,6 +5,7 @@ import (
 	"context"


It's an in-memory only thing?

sanderpick · 2020-04-24T15:22:02Z

ffs/coreipfs/coreipfs.go

@@ -106,6 +107,12 @@ func (ci *CoreIpfs) Store(ctx context.Context, c cid.Cid) (int, error) {
 	if err != nil {
 		return 0, fmt.Errorf("getting stats of cid %s: %s", c, err)
 	}
+	if err := ci.ensurePinsetCache(ctx); err != nil {


Could this just be done on startup?

Yep, that's an option. But I tried to avoid adding load to bootstrapping and just load it on first use. But this can perfectly be moved to New.

Cool, makes sense. I guess the only downside of having it here is that there will one (the first after start) lucky user that gets a slow response time.

Yeah, that's a good point.
The other option is spinning a go routine on New... "nobody" pays a cost. :P

Oh yeah, sounds good 💯

sanderpick · 2020-04-24T15:23:41Z

ffs/integrationtest/integration_test.go

@@ -989,6 +1007,67 @@ func TestRemove(t *testing.T) {
 	require.Equal(t, api.ErrNotFound, err)
 }

+// This isn't very nice way to test for repair. The main problem is that now
+// deal start is buffered for future start for 10000 blocks at the Lotus level.


jsign · 2020-04-24T15:30:34Z

LGTM!

Thanks for the review!

jsign added 3 commits April 22, 2020 17:50

progress on repair before rebase

c3942f2

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

ffs: repair logic

ec4a894

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

ffs/scheduler: repair test

5b9d349

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign added the ffs label Apr 22, 2020

jsign added this to the Sprint 35 milestone Apr 22, 2020

jsign self-assigned this Apr 22, 2020

lints

9c4f776

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign added 10 commits April 22, 2020 19:48

more lint

6f17d9b

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

fix

d05c54b

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

update lotus-client dep

4f34cc9

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

update to new interopnet

0a6101a

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

ffs: re-enable three deal making with single miner

7833149

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

updated lotus-devnet and unskip more tests

30480e1

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

disable verbose logging in integration test

08b153b

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

fix epochprice test

b25907a

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

keep skipped one

22d3979

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

lints

94ea9e0

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign changed the title ~~ffs: enable auto-repair~~ ffs: enable auto-repair & update to new interopnet Apr 23, 2020

ffs/coreipfs: make pinset cache

813653c

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign changed the title ~~ffs: enable auto-repair & update to new interopnet~~ ffs: enable auto-repair & update to new interopnet & ipfs pinset cache Apr 24, 2020

fix pinset cache

415e4d4

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign/repair branch 2 times, most recently from 0c50452 to 23879be Compare April 24, 2020 11:01

ffs/integrationtest paralellized && disable meaningless test

6229ab5

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign force-pushed the jsign/repair branch from 23879be to 6229ab5 Compare April 24, 2020 11:42

avoid ask index test in new interop

b102de0

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>

jsign commented Apr 24, 2020

View reviewed changes

jsign requested a review from asutula April 24, 2020 12:13

jsign marked this pull request as ready for review April 24, 2020 12:13

jsign requested a review from sanderpick April 24, 2020 12:14

asutula approved these changes Apr 24, 2020

View reviewed changes

jsign merged commit b585b89 into master Apr 24, 2020

jsign deleted the jsign/repair branch April 24, 2020 15:04

sanderpick reviewed Apr 24, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ffs: enable auto-repair & update to new interopnet & ipfs pinset cache #272

ffs: enable auto-repair & update to new interopnet & ipfs pinset cache #272

jsign commented Apr 22, 2020 •

edited

Loading

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

sanderpick Apr 24, 2020

sanderpick Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

jsign Apr 24, 2020

asutula left a comment

asutula Apr 24, 2020

jsign commented Apr 24, 2020

sanderpick left a comment

sanderpick Apr 24, 2020

sanderpick Apr 24, 2020

sanderpick Apr 24, 2020

jsign Apr 24, 2020

sanderpick Apr 24, 2020

jsign Apr 24, 2020

sanderpick Apr 24, 2020

sanderpick Apr 24, 2020

jsign commented Apr 24, 2020

		"/dns4/t01000.miner.interopnet.kittyhawk.wtf/tcp/1347/p2p/12D3KooWNjqGTNZ592wG5DmX7Rbmb76V3NHMdZMqjqPJ1xiax7w6",
		"/ip4/54.186.82.90/tcp/1347/p2p/12D3KooWGf4ggd3cdZJKZPHkgSe8Lxf4MDsKhVC7npRGXgsPP4fz",

		p := mp.MinerPower.Uint64()
		p := mp.MinerPower.RawBytePower.Uint64()

ffs: enable auto-repair & update to new interopnet & ipfs pinset cache #272

ffs: enable auto-repair & update to new interopnet & ipfs pinset cache #272

Conversation

jsign commented Apr 22, 2020 • edited Loading

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

Choose a reason for hiding this comment

asutula left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsign commented Apr 24, 2020

sanderpick 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

jsign commented Apr 24, 2020

jsign commented Apr 22, 2020 •

edited

Loading