retrieval: Direct piecereader

[magik6k]

Cherry-picks parts of #5769, and makes them work

Did some testing in a pond devnet with a worker, it seems to work, but needs testing on a real node, and ideally some integration tests

[dirkmc]

Looks good, just one question

[arajasek]

Nothing seems very wrong, and I think I understand what it does. May even work.

[dirkmc]

When we tried to run this with our miner we got the following error when doing a retrieval:

2021-05-04T18:32:57.675+0300    INFO    markets loggers/loggers.go:30   retrieval provider event        {"name": "ProviderEventCancelComplete", "deal ID": "1620120981223977232", "receiver": "12D3KooWHMGTcNbS64arHyaPoUv6NrZi9V63kuVbnE6jbBjm5EUS", "state": "DealStatusErrored", "message": "failed to unseal piece from sector 647: opening partial file: file '/root/storage_sealing/unsealed/s-t0127896-647' has inconsistent length; has 34359738377 bytes; expected 2057 (9 trailer, 2048 sector data)"}

[magik6k]

^ likely caused by not setting the seal proof type in the http handler / remote store on either the worker or miner node

[aarshkshah1992]

@magik6k Thanks for that pointer. I'll grok this PR, factor in your suggestion and take this work to completion as we really need it to fix data retrievals.

Btw, do you have any pointers to existing integration tests that we can use as reference to write integration tests for this feature ?

[magik6k]

Btw, do you have any pointers to existing integration tests that we can use as reference to write integration tests for this feature ?

Api tests which do retrieval (like this one) should be exercising most of the now code here (I'd run them with coverage to see if that's actually the case).

Testing the remote parts / new http code is a bit more complex as it requires setting up workers in the test framework, which is currently not there, and will probably require refactoring the worker constructor to be usable in tests (cmd/lotus-seal-worker/main.go), possibly by making it use the DI framework (I could help with this - if not, having it be non-DI based for now is also fine, the worker isn't that complicated)

[magik6k]

(for worker tests, basically we'd need to extract the whole worker thing from the run command function, and make it possible to provide things which are now taken from cctx (miner API, miner store address, temp repo) from a test)

[magik6k]

I think this is the issue:

• UnsealSector is getting sector info from lower-level secb.SectorsStatus (storage-fsm), which now returns api.SectorInfo (so that the interface matches the RPC interface for when we separate the market node)
• SectorInfo.SealProof is an optional field filled out in higher-level RPC impl, set to zero in storage-fsm -> https://github.com/filecoin-project/lotus/pull/6128/files#diff-0dd21b9cbecc38ac24b8e03f24c12f67fe8e5620af9bcca340a647017897135dR80
• We then pass that zero-value in retrieval adapter -> https://github.com/filecoin-project/lotus/pull/6128/files#diff-3b8c5ae92101b90e9b36a390ea8bcd193b7d519a667006a3d514ac163a76afaeR69
• That value eventually gets used in the http_handler to get the sector size -> https://github.com/filecoin-project/lotus/pull/6128/files#diff-578f861bfe2bd7f0ffb0f6b853915dd65fb4ffdc0dcf0c2825948010fcb321e3R166 (sealProof=0 is old 2k sectors, which is why this worked for me in pond..)

I think the correct thing to do here is to just set the proof type in the fsm adapter from info.SectorType (which is the first field set after sectors are created, so it should be set no matter at which stage of the sealing pipeline sectors are)

[magik6k]

Superseded by #6280