Skip to content

Plan: smlbench #60

Merged
daviddias merged 7 commits intomasterfrom
plan-smlbench
Oct 20, 2019
Merged

Plan: smlbench #60
daviddias merged 7 commits intomasterfrom
plan-smlbench

Conversation

@daviddias
Copy link
Copy Markdown
Contributor

Goal: get smlbench to work through testground

_ = os.Setenv("TEST_CASE", tc.Name())
_ = os.Setenv("TEST_CASE_SEQ", strconv.Itoa(i))

// ctx := api.NewContext(context.Background())
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raulk What did you intend to do with this api.NewContext? I mean, where it is supposed to come from.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just dawned on me that probably all I need is to do this https://github.com/ipfs/go-ipfs/pull/6695/files#diff-5146146b33d1d2368a717a97d254547dR151. Will test in a sec

This was referenced Oct 7, 2019
@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 16, 2019

It's a little unclear on how the new version of the smlbench should interact with Docker...

The old version didn't use Docker, and simply used the wrapper around IPTB from the testground SDK + the version of IPFS found in the path.

There does appear to be an IPTB plugin for Docker ... but I assume that's not what we want to use, as testground already spins up Docker instances (eg. you pass in the number of desired instances for the DHT test).

The simple_add test only runs on a single IPFS node (spawned by IPTB).

The simple_add_get test uses IPTB to spawn an "adder" node and a "getter" node.

The simplest thing would be to get IPTB built inside a single Docker image, and run both the "adder" and "getter" via IPTB in a single Docker container.

The testground SPEC mentions IPTB several times in order to explain concepts, but doesn't specifically say that we are using it.

https://github.com/ipfs/testground/blob/master/docs/SPEC.md#overview

For some future tests, we want to be able to use things like tc to do traffic shaping, which suggests that we want each node in a separate Docker container.

We could use an approach like the DHT tests, where the test binary is launched in separate Docker instances...

https://github.com/ipfs/testground/blob/master/plans/dht/test/lookup_peers.go

For the "simple_add_get" test, one possibility could be to run the test with the command-line argument --instances=2, and have each instance decide if it's the "adder" or the "getter". It looks like the Docker container name contains a incrementing counter for each instance - it might not be passed down via environment variables though to the runenv...

If we wanted to preserve the IPTB api for imperatively creating instances from within tests, but create them in separate Docker instances, that might be possible by making an IPTB plugin that communicated back to the 'engine'. I'm not sure if we want to do that or not.

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 16, 2019

The easiest thing just to get the test to pass is going to be to run the existing IPTB-based tests in a single Docker container ... I'll prototype that on a branch and link it here when it's working.

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 17, 2019

It wasn't the easiest thing to get working, but here it is:

https://github.com/ipfs/testground/tree/plan-smlbench-single-container

Try:

./testground -vv run --builder docker:go --runner local:docker --instances=1 smlbench/simple-add

or

./testground -vv run --builder docker:go --runner local:docker --instances=1 smlbench/simple-add-get

I had to do a lot of surgery to the Dockerfile to (optionally) download go-ipfs. I switched the base image from busybox to Debian because the prebuild go-ipfs needs a full glibc.

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 17, 2019

That's just a first stab at it ... I'm not entirely satisfied with how I modified the Dockerfile template to add IPFS to it. It probably makes more sense to use different templates depending on the test.

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 17, 2019

Another thought ... for published IPFS binaries, we could copy them out of an existing Docker image, which would cache a bit more nicely than downloading them each time.

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 17, 2019

I'm done for the night... so I'll leave this here to see if I'm on the right track...

I started another branch:

https://github.com/ipfs/testground/tree/plan-smlbench-two-containers

I made a "smlbench2" test suite, which is just "smlbench", but only with the simple_add_get test enabled. I'm going to try to spin it up in 2 different docker containers - and have one assume the "adder" role, and the other assume the "getter" role.

If I understand correctly, I can get a sequence number from redis by calling writer.Write() https://github.com/ipfs/testground/blob/944d3429bcf06137d0754ba3870ae35eb5504d55/sdk/sync/write.go#L109

If that works the way I hope it does, one peer can act as the "adder" and the other as the "getter". Once the peers are ready, I just need to connect them. Then I need to add the file on the adder (smlbench already has this), and signal the state, then get it on the "getter".

@daviddias
Copy link
Copy Markdown
Contributor Author

daviddias commented Oct 17, 2019

@jimpick lot's of progress, nice! Could you open a PR against this one so that I can see exactly the differences and make comments inline?

testCases := testCasesSet[runenv.TestCaseSeq]

for i, tc := range testCases {
_ = os.Setenv("TEST_CASE", tc.Name())
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hitting:

# github.com/ipfs/testground/plans/smlbench
./main.go:67:32: tc.Name undefined (type utils.SmallBenchmarksTestCase has no field or method Name)

Although the method is declared. Means that I'm missing some goism here. Will figure this out

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this line to add .Name() to the interface

https://github.com/ipfs/testground/blob/plan-smlbench-single-container/plans/smlbench/utils/testcase.go#L10

My guess is that when it is compiling in the container, it doesn't have the latest code... I wonder if it's pulling from github instead of copying the local source code into the docker image?

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 17, 2019

@jimpick lot's of progress, nice! Could you open a PR against this one so that I can see exactly the differences and make comments inline?

Here it is:

#73

This PR is only for reference ... feel free to incorporate anything from it. I'm also working on a different approach that uses two Docker containers and the Testground Sync SDK, which I'm guessing is closer to what we want to do.

@jimpick
Copy link
Copy Markdown
Contributor

jimpick commented Oct 20, 2019

I've got the "2 container" version of the smlbench testplan (simple-add-get) partially working...

See #74 for all the details.

It's using the Testground Sync API to communicate between the peers. I'll have to tweak it a little bit to get it to work in Docker, but I'm really close.

* Get smlbench to compile (no tests)

* Include go-ipfs in Docker image when go_ipfs_version is defined

If not defined, Docker build still works.

* Get smlbench working again

Changed Docker base image to Debian because busybox or alpine don't
work with our precompiled ipfs binaries.

Also added a small delay when destroying the ensemble to avoid a
race.
@daviddias daviddias marked this pull request as ready for review October 20, 2019 15:42
@daviddias
Copy link
Copy Markdown
Contributor Author

@jimpick thank you so much for the help in landing this one :) With this and having done https://github.com/ipfs/go-ipfs/pull/6695/files, I now feel fully unblocked to land #58

@daviddias daviddias merged commit a6bc997 into master Oct 20, 2019
@daviddias daviddias deleted the plan-smlbench branch October 20, 2019 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants