Initial changes to support go-ipfs #268

jimpick · 2019-07-31T04:57:39Z

Ready to merge. :-)

Separate js-ipfs and go-ipfs Grafana dashboards
New required param "project" in REST API POST to add a test
(either 'js-ipfs' or 'go-ipfs')
Temporary hack to reduce tests to a single test to speed up
testing (localTransfer_tcp_mplex_secio)

"project" setting is not passed through yet. Will probably need
modifications to ansible scripts and some new code to rebuild
go-ipfs to match requested sha hash. Test may need modifications,
and upload to InfluxDB will need to set the right metadata.

Work-in-progress ... do not merge yet * Separate js-ipfs and go-ipfs Grafana dashboards * New required param "project" in REST API POST to add a test (either 'js-ipfs' or 'go-ipfs') * Temporary hack to reduce tests to a single test to speed up testing (localTransfer_tcp_mplex_secio) "project" setting is not passed through yet. Will probably need modifications to ansible scripts and some new code to rebuild go-ipfs to match requested sha hash. Test may need modifications, and upload to InfluxDB will need to set the right metadata.

I was having trouble getting go-ipfs to connect to js-ipfs in the tests ... the libp2p config wasn't passing in the secio object in the right place.

v0.4.10 -> v0.4.21

The script was failing when it tried to delete the non-existent symlink, and subsequent runs would not recreate it.

Also fixes unit tests in 'runner'.

jimpick · 2019-08-01T06:27:37Z

I fixed up the tests that were using go-ipfs. I think it needs a switch that tells it which set of tests to run:

js-ipfs from a sha hash + stable go-ipfs release + relevant tests -> results go to 'js' dashboard
go-ipfs from a sha hash + stable js-ipfs + relevant tests -> results go to 'go' dashboard

We could run an arbitrary js-ipfs hash vs. an arbitrary go-ipfs hash, but we'd have to figure out the dashboard situation, if we stick all the results on a single dashboard, it might be a bit of a mishmash.

There's only one test currently that involves go-ipfs by itself (local-add.go.js). The other two tests are interop between js-ipfs and go-ipfs (extract-go2.js and extract-js2.go.js). It doesn't appear that any of these tests were currently active.

The only reason I can think of for running local-add.go.js during the js-ipfs test would be to have a comparative number. In that case, it should probably be displayed on the 'js' dashboard alongside the other tests run at the same time.

I'm pretty close to having the switch where we can build a custom go version, and run the 3 tests that we have against it.

Stebalien · 2019-08-01T23:45:46Z

Separate js-ipfs and go-ipfs Grafana dashboards

Why are we splitting these into two dashboards and not just labeling the test runs? We should be able to run the current js-ipfs benchmarks against go-ipfs.

Was causing chicken-and-egg bootstrapping problem.

My ssh key was in a different location, remove hardcoded "elexy".

When creating tests, each test can be specified to be a default for a list of different targets. Also, throw an error if the provision step fails.

To make it easier to debug.

Tests now get a 'meta' object so we can add more fields in the future.

jimpick · 2019-08-02T18:25:26Z

@Stebalien It will probably make more sense when I demo it. With Grafana, typically you have lots of dashboards corresponding to different views on the data - I'm just tagging the data so you can tell what came from where.

I'm just starting to learn how to use InfluxDB + Grafana ... here are some screenshots from my dev setup - I've barely started to explore the possibilities:

Stebalien · 2019-08-02T18:45:59Z

Got it. My main concerns are:

The benchmarks themselves will often run against both go and js.
I'd like to be able to compare go and JS.

That's why I'm thinking this should be more of a filter than anything (or maybe two dashboards and a combined dashboard).

jimpick · 2019-08-02T19:22:23Z

I think that's pretty easy to do, I just need to figure out how to build the query. :-)

Here's another dashboard with separate graphs for each test, with different lines for each file size - I think it's a bit easier to grok:

It's possible to overlay all sorts of information and also provide ways to "drill down".

We now build both js-ipfs and go-ipfs from github master each time. The support for specific 'commit' tests are currently broken, as the scripts will attempt to checkout the same commit on both js-ipfs as well as go-ipfs ... we'll need to switch on the target as well as pick a default for the other. I've hardcoded some paths for the go-ipfs binary we build ... that should be fixed. I added an ansible role to install an up-to-date version of go. There are multiple versions of go and ipfs on the system, so it's a bit hazardous as the wrong version might be used by accident.

For some reason, 'make install' didn't build the binary as expected.

jimpick · 2019-08-03T06:14:01Z

It now seems to be successfully building a "fresh" js-ipfs and go-ipfs for each run off of master. I need to fixup manually selecting a sha for a job. There is more work to do to make sure the sha for both js-ipfs and go-ipfs gets through to InfluxDB.

I experimented with using "Annotations" with Grafana to show git hashes on the graph. I will need to add some code to maintain that table in the database.

…go-ipfs-support

Only 3 tests currently.

tests/lib/create-node.js

alanshaw · 2019-08-06T16:07:22Z

infrastructure/playbooks/benchmarks.yaml

        rm -Rf go-ipfs && \
        chmod +x /usr/local/bin/ipfs_{{go_ipfs_version}} && \
-        rm /usr/local/bin/ipfs && \
+        rm -Rf /usr/local/bin/ipfs && \


How come this is needed now?

The rm fails if /usr/local/bin/ipfs is not there already, which it won't be on a non-bootstrapped system. It should probably just be rm -f ... the recursive option is not needed since it's a single file.

alanshaw · 2019-09-05T10:02:37Z

runner/lib/configBenchmarks.js

    name: 'localTransfer_tcp_mplex',
-    file: 'local-transfer.js -t tcp -m mplex'
+    file: 'local-transfer.js -t tcp -m mplex',
+    defaultFor: [ 'js-minion' ]


Do you think this would be better named targets?

alanshaw · 2019-09-05T10:04:19Z

runner/lib/schema/add.js

+    },
+    target: {
+      type: 'string',
+      description: 'currently js-minion or go-minion',


Please can we have a better description? It is already out of date (can be js-minion-lite)!

Yeah, that needs a better description.

I'm not 100% satisfied with the name of the option - i called it "target" because i didn't have a better idea at the moment - it's not really a test suite, since both js and go might share tests, and js-minion-lite is a subset of the js-minion tests. "Target" is used by gcc to refer to a combination of cpu + operating system when cross-compiling, but this is a bit different, since it's referring to the implementation.

alanshaw · 2019-09-05T10:07:50Z

runner/provision.js

+    config.log.info(jsOut)
+    // let goOut = await remote.run(`${config.benchmarks.remotePath}getGoIpfs.sh ${config.ipfs.path} ${commit || ''}`)
+    let goOut = await remote.run(`${config.benchmarks.remotePath}getGoIpfs.sh ${config.ipfs.path}`)
+    config.log.info(goOut)


If I'm reading this correctly the passed commit is ignored now?

I think I need to take a second pass and figure out a better solution for this.

I didn't like that 'commit' doesn't specify whether or not it's referring to go-ipfs or js-ipfs. Right now, I'm just building both from master every time.

Testing master against master for the one js <-> go interop test might be the right thing to do. It might make more sense to test js-ipfs master vs. a stable release of go-ipfs (latest one?) and go-ipfs vs. a stable release of js-ipfs.

I don't think the 'commit' "manual override" feature is actively being used, but I think it might be good to be able to pass in a separate commit for js-ipfs and go-ipfs at the same time for maximum flexibility.

alanshaw · 2019-09-05T10:24:29Z

runner/runner.js

    try {
      await mkDir(`${targetDir}/${test.name}`, { recursive: true })
-      let arrResult = await runCommand(test.benchmark, test.name)
+      let arrResult = await runCommand(test.benchmark + ` --target=${params.target}`, test.name)


Suggested change

let arrResult = await runCommand(test.benchmark + ` --target=${params.target}`, test.name)

let arrResult = await runCommand(test.benchmark + (params.target ? ` --target=${params.target}` : '', test.name)

alanshaw · 2019-09-05T10:27:23Z

tests/extract-js2.go.js

  name = protocal === 'ws' ? `${name}Ws` : name
  const id = protocal === 'ws' ? 2 : 0
-  let command = `export IPFS_PATH=${conf.tmpPath}/ipfs0 && ipfs swarm connect ${peerId.addresses[id]}`
+  let command = `export IPFS_PATH=${conf.tmpPath}/ipfs0 && /home/ubuntu/ipfs/go-ipfs/cmd/ipfs/ipfs swarm connect ${peerId.addresses[id]}`


alanshaw · 2019-09-05T10:32:26Z

tests/getGoIpfs.sh

+  git clone https://github.com/ipfs/go-ipfs.git 2>&1
+  cd go-ipfs
+else
+  echo "> Git repo for js-ipfs found, updating..."


Suggested change

echo "> Git repo for js-ipfs found, updating..."

echo "> Git repo for go-ipfs found, updating..."

alanshaw · 2019-09-05T10:32:56Z

tests/getGoIpfs.sh

+  git config --global advice.detachedHead false
+  git checkout $COMMIT 2>&1
+fi
+echo "run make build for go-ipfs"


Suggested change

echo "run make build for go-ipfs"

echo "> running make build for go-ipfs"

alanshaw · 2019-09-05T10:34:48Z

tests/lib/create-node.js

  await initRepo(peerDir)
  await fsWriteFile(`${peerDir}/config`, JSON.stringify(peerConf))
-  let peer = spawn('ipfs', ['daemon'], { env: Object.assign(process.env, { IPFS_PATH: peerDir }) })
+  let peer = spawn('/home/ubuntu/ipfs/go-ipfs/cmd/ipfs/ipfs', ['daemon'], { env: Object.assign(process.env, { IPFS_PATH: peerDir }) })


Needs to be global or configuration passed that has this path!

alanshaw · 2019-09-05T10:49:18Z

tests/schema/results.js

    'project': 'js-ipfs',
    'commit': 'TBD',
-    'version': 'version of js-ipfs'
+    'version': 'version of js-ifps',


Suggested change

'version': 'version of js-ifps',

'version': 'version of js-ipfs',

alanshaw · 2019-09-06T17:02:54Z

ping @jimpick

jimpick · 2019-09-07T17:00:47Z

Sorry for the delay responding ... I just added a gmail filter so hopefully i don't overlook these things from now on. I'll respond to the line-by-line comments now.

I was planning to do another pass on this, but I believe @raulk wants the go-ipfs tests in https://github.com/ipfs/testground instead, so I hadn't invested the extra time.

testground/testground#25

I don't think there would be any harm in deploying these changes, but they were just a first step ... they don't make much sense unless we also write more go test cases.

jimpick · 2019-09-07T18:24:35Z

I'll also add that I also made a bunch of grafana dashboards where I re-organized the graphs to present the data in multiple dashboards in a more useable layout. I also did some experimentation in adding annotation data so that Grafana can display when the github commits occurred so it's easier to reason about the graphs you are looking at.

Those new dashboards and annotations are probably useful even if we don't run the go-ipfs tests on the ipfs/benchmarks minion. I can dump the dashboards out as JSON, but it's probably easier to discuss if I actually put them up somewhere publicly accessible. Right now, they only exist in the Grafana instance on my development desktop.

jimpick and others added 6 commits July 30, 2019 21:50

Fix connEncryption param in tests

46dc4b6

I was having trouble getting go-ipfs to connect to js-ipfs in the tests ... the libp2p config wasn't passing in the secio object in the right place.

Upgrade version of go-ipfs used in js-ipfs tests

11ea88e

v0.4.10 -> v0.4.21

Ensure /usr/local/bin/ipfs symlink is installed

eedfbd3

The script was failing when it tried to delete the non-existent symlink, and subsequent runs would not recreate it.

Merge remote-tracking branch 'origin/patch-4' into go-ipfs-support

d3a8877

Re-enable all tests

c020c55

Also fixes unit tests in 'runner'.

jimpick added 7 commits August 1, 2019 17:19

"build-browser" depends on having a checked out js-ipfs

7082f9d

Was causing chicken-and-egg bootstrapping problem.

Override hardcoded values with BENCHMARK_USER and BENCHMARK_KEY

5f25f8f

My ssh key was in a different location, remove hardcoded "elexy".

Add "target" param to switch between "js-minion" and "go-minion"

a241a66

When creating tests, each test can be specified to be a default for a list of different targets. Also, throw an error if the provision step fails.

Each output from go-ipfs stdout/stderr

e14b951

To make it easier to debug.

Revert accidental commit

364c341

Pass 'target' all the way through to the results and grafana

aecd537

Tests now get a 'meta' object so we can add more fields in the future.

Add js-minion-lite target for quick dev/testing

f182c7b

jimpick added 2 commits August 2, 2019 22:13

Use 'make build' when building go-ipfs

4d9eb1d

For some reason, 'make install' didn't build the binary as expected.

Jim Pick and others added 4 commits August 5, 2019 07:40

Merge branch 'master' into go-ipfs-support

341780c

Merge branch 'master' into go-ipfs-support

b39ffcf

Merge branch 'go-ipfs-support' of github.com:jimpick/benchmarks into …

8c5ed68

…go-ipfs-support

Also run the go-minion target every night

f7cb65d

Only 3 tests currently.

raulk reviewed Aug 6, 2019

View reviewed changes

tests/lib/create-node.js Show resolved Hide resolved

alanshaw requested changes Sep 5, 2019

View reviewed changes

	let arrResult = await runCommand(test.benchmark + ` --target=${params.target}`, test.name)
	let arrResult = await runCommand(test.benchmark + (params.target ? ` --target=${params.target}` : '', test.name)

	echo "> Git repo for js-ipfs found, updating..."
	echo "> Git repo for go-ipfs found, updating..."

	echo "run make build for go-ipfs"
	echo "> running make build for go-ipfs"

	'version': 'version of js-ifps',
	'version': 'version of js-ipfs',

Initial changes to support go-ipfs #268

Are you sure you want to change the base?

Initial changes to support go-ipfs #268

Uh oh!

Conversation

jimpick commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimpick commented Aug 1, 2019

Uh oh!

Stebalien commented Aug 1, 2019

Uh oh!

jimpick commented Aug 2, 2019

Uh oh!

Stebalien commented Aug 2, 2019

Uh oh!

jimpick commented Aug 2, 2019

Uh oh!

jimpick commented Aug 3, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanshaw commented Sep 6, 2019

Uh oh!

jimpick commented Sep 7, 2019

Uh oh!

jimpick commented Sep 7, 2019

Uh oh!

Uh oh!

jimpick commented Jul 31, 2019 •

edited

Loading