Implement Visor Vector Builder & Executor #370

frrist · 2021-01-26T00:09:33Z

What

This PR adds vector creation and execution logic to Visor. A README with example usage can be viewed here: https://github.com/filecoin-project/sentinel-visor/pull/370/files?short_path=405134e#diff-405134e8ddb86a9f71f27944fd2349c9f86062c1797bce502dcfb32fb36f693f. There is a lot of room for improvement here, but given this PR's age, the churn from the past couple weeks, and changing priorities, I wanted to put a pin in this. I intend to make further improvements to this as follow-ons.

How to review

Look over the README linked above, then read commit by commit.

@iand I have created 85eb065df81f0ab2688530772c72f078344c9bab to serve as a strawman for starting a conversation on how to fix #374.

Here is a demo showing how to create and execute visor test vectors.

frrist · 2021-02-17T00:56:26Z

.gitignore

@@ -19,3 +19,4 @@ visor
 sentinel-visor

 build/.*
+vector/data/*.json


Ensure the vector files don't get pushed to github.

frrist · 2021-02-17T01:01:08Z

lens/vector/repo.go

+	// wrap the repos blockstore with a tracing implementation capturing all cid's read.
+	capi.tbs = NewTracingBlockstore(bs)


Wrapping the API's blockstore with a TracingBlockstore tracks CIDs read from it. The recorded CIDs are used to build the vectors CAR data.

frrist · 2021-02-17T01:03:05Z

storage/mem.go

@@ -0,0 +1,63 @@
+package storage


Implement the storage interfaces backed by a map.

vector/fetch_vectors.sh

frrist · 2021-02-17T01:06:46Z

vector/runner.go

+}
+
+func modelTypeFromTable(tableName string, expected json.RawMessage, actual []interface{}) (string, error) {
+	// TODO: something with reflection someday


It was faster for me to just write this all out than tinker with reflection.

This method validates the data extracted while executing the test vector matches the expectation of the vector.

iand

Generally LGTM. Mostly cosmetic comments.

iand · 2021-02-18T12:34:38Z

chain/indexer.go

 		ll.Debugw("tipset complete", "total_time", time.Since(start))
 	}()
+	wg.Wait()


This will force the extraction and persistence to be locked in step, but they are supposed to be out of step. The idea is that we can be extracting a tipset while persisting the previous one (which is mostly IO wait).

Can you take this out and let's figure out a fix for #374 separately

I have reverted this change here: 288beafa4673e74bbaf3fa024799254f06e61f1e
And applied a hacky-fix here: fb47362dbd11c562b63478b001f871691d0ef6f7

I agree this, nor the hack are the correct solution, but without one of them all vector tests fail. I believe this is because once the walker has indexed the last tipset in its walk it calls this method and exists regardless of this go routine completing its persistence work.

commands/setup.go

commands/vector.go

lens/vector/tracing_bs.go

iand · 2021-02-18T12:44:46Z

storage/mem.go

@@ -0,0 +1,63 @@
+package storage


iand · 2021-02-18T12:49:45Z

vector/README.md

+```
+Finally, execute the vector unit tests:
+```
+$ go test -v ./vector/...


Tried these commands out and it all worked smoothly. 🏆

haha, spoke too soon. One vector failed to download correctly and the file contains

<html> <head><title>429 Too Many Requests</title></head> <body> <center><h1>429 Too Many Requests</h1></center> <hr><center>openresty</center> </body> </html>

Damn, ideally the curl command in fetch_vectors.sh would be ipfs or ipget but I didn't want to muck with those in CI just yet. I have modified fetch_vectors.sh to fail in the event curl fails, and I think that should address this... 🥈

vector/fetch_vectors.sh

Users may use the vector lens to capture all CID's written to and read from its API. The lens may be used in conjunction with the vector commands to produce a test vector.

Implement vector build and vector runner commands. These commands allow a user to build and execute vectors against Visor.

This reverts commit 85eb065df81f0ab2688530772c72f078344c9bab.

- will revert after a proper fix is designed.

frrist · 2021-02-18T23:43:43Z

Need to regenerate the message vector after the rebase, then should be ready for merge.

codecov-io · 2021-02-19T13:53:43Z

Codecov Report

Merging #370 (6dcf1ec) into master (daf7e2b) will decrease coverage by 2.5%.
The diff coverage is 34.1%.

@@           Coverage Diff            @@
##           master    #370     +/-   ##
========================================
- Coverage    46.4%   43.9%   -2.6%     
========================================
  Files          24      29      +5     
  Lines        1796    2258    +462     
========================================
+ Hits          835     993    +158     
- Misses        834    1083    +249     
- Partials      127     182     +55

frrist changed the title ~~wip: add camera lens~~ Visor Vector WIP Jan 26, 2021

placer14 mentioned this pull request Jan 26, 2021

Verify the accuracy of visor data in the analysis schema #372

Open

frrist force-pushed the frrist/vector-builder branch 3 times, most recently from b2e13e4 to 17c8329 Compare February 16, 2021 22:14

frrist changed the title ~~Visor Vector WIP~~ Visor Vector Builder Feb 16, 2021

frrist self-assigned this Feb 16, 2021

frrist commented Feb 17, 2021

View reviewed changes

vector/fetch_vectors.sh Show resolved Hide resolved

frrist commented Feb 17, 2021

View reviewed changes

frrist marked this pull request as ready for review February 17, 2021 01:14

filecoin-project deleted a comment from codecov-io Feb 17, 2021

frrist requested review from iand and placer14 February 17, 2021 01:16

frrist force-pushed the frrist/vector-builder branch from 2d2aa0b to 8646a14 Compare February 17, 2021 01:25

iand approved these changes Feb 18, 2021

View reviewed changes

frrist force-pushed the frrist/vector-builder branch 2 times, most recently from fb47362 to e6370b8 Compare February 18, 2021 22:42

frrist added 8 commits February 18, 2021 14:48

feat(vector lens): implement vector lens

2c82e6c

Users may use the vector lens to capture all CID's written to and read from its API. The lens may be used in conjunction with the vector commands to produce a test vector.

feat(vector): implement vector build and run

e5fdf5d

Implement vector build and vector runner commands. These commands allow a user to build and execute vectors against Visor.

temp-fix: strawman for #374

f1ab952

fix(model): PowerActorClaim missing from model list

9dc646c

polish(vector): add vector fetching script and unit test

0733e65

docs(vector): add readme with example usage

ee65592

Revert "temp-fix: strawman for #374"

4a9b539

This reverts commit 85eb065df81f0ab2688530772c72f078344c9bab.

hack: hacky fix for #374

69afaa0

- will revert after a proper fix is designed.

frrist force-pushed the frrist/vector-builder branch from e6370b8 to 69afaa0 Compare February 18, 2021 23:46

frrist and others added 2 commits February 18, 2021 17:34

test(vector): generate new message vector after rebase onto #389

176f1aa

fix lint errors

6dcf1ec

frrist changed the title ~~Visor Vector Builder~~ Implement Visor Vector Builder & Executor Feb 19, 2021

frrist merged commit dcc5517 into master Feb 19, 2021

frrist deleted the frrist/vector-builder branch February 19, 2021 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Visor Vector Builder & Executor #370

Implement Visor Vector Builder & Executor #370

frrist commented Jan 26, 2021 •

edited

Loading

frrist Feb 17, 2021

frrist Feb 17, 2021

frrist Feb 17, 2021

iand Feb 18, 2021

frrist Feb 17, 2021 •

edited

Loading

iand left a comment

iand Feb 18, 2021

frrist Feb 18, 2021

iand Feb 19, 2021

iand Feb 18, 2021

iand Feb 18, 2021

iand Feb 18, 2021

frrist Feb 18, 2021 •

edited

Loading

frrist commented Feb 18, 2021

codecov-io commented Feb 19, 2021

		// wrap the repos blockstore with a tracing implementation capturing all cid's read.
		capi.tbs = NewTracingBlockstore(bs)

Implement Visor Vector Builder & Executor #370

Implement Visor Vector Builder & Executor #370

Conversation

frrist commented Jan 26, 2021 • edited Loading

What

How to review

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

frrist Feb 17, 2021 • edited Loading

Choose a reason for hiding this comment

iand 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

frrist Feb 18, 2021 • edited Loading

Choose a reason for hiding this comment

frrist commented Feb 18, 2021

codecov-io commented Feb 19, 2021

Codecov Report

frrist commented Jan 26, 2021 •

edited

Loading

frrist Feb 17, 2021 •

edited

Loading

frrist Feb 18, 2021 •

edited

Loading