Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add api for transfer diagnostics #7759

Merged
merged 5 commits into from
Dec 22, 2021
Merged

Conversation

hannahhoward
Copy link
Contributor

Goals

We've done some work in data transfer and go-graphsync to expose detailed diagnostic information for what's going on with data transfers over graphsync.

This PR adds an API endpoint + CLI for accessing that information in Lotus.

Implementation

  • refactor storage miner node modules a bit to allow injecting the Data Transfer transport as a module
  • rename NewProviderDAGServiceDataTransfer as this should have been renamed a while ago
  • add MarketsDataTransferDiagnostics method to API
    • implement with calls to graphsync & data transfer graphsync transport
  • add CLI command

Sample output of what this command produces (it's just JSON for now):

{
  "sendingTransfers": null,
  "receivingTransfers": [
    {
      "requestID": 4570,
      "requestState": "running",
      "isCurrentChannelRequest": true,
      "channelID": {
        "Initiator": "12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd",
        "Responder": "12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe",
        "ID": 1639108958367248600
      },
      "channelState": {
        "selfPeer": "12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd",
        "remotePeer": "12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe",
        "status": 1,
        "statusMessage": "Ongoing",
        "sent": 2161269155,
        "received": 0,
        "message": "",
        "baseCid": "bafybeiaolzzjpxq3iyp7gwk4qzzbmmybkwjsudzjsroxatgdqmvy7auqgy",
        "channelId": {
          "Initiator": "12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd",
          "Responder": "12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe",
          "ID": 1639108958367248600
        }
      },
      "diagnostics": null
    },
    {
      "requestID": 4071,
      "requestState": "no graphsync state found",
      "isCurrentChannelRequest": false,
      "channelID": {
        "Initiator": "12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd",
        "Responder": "12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe",
        "ID": 1639108958367248600
      },
      "channelState": {
        "selfPeer": "12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd",
        "remotePeer": "12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe",
        "status": 1,
        "statusMessage": "Ongoing",
        "sent": 2161269155,
        "received": 0,
        "message": "",
        "baseCid": "bafybeiaolzzjpxq3iyp7gwk4qzzbmmybkwjsudzjsroxatgdqmvy7auqgy",
        "channelId": {
          "Initiator": "12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd",
          "Responder": "12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe",
          "ID": 1639108958367248600
        }
      },
      "diagnostics": [
        "No current request state for data transfer channel id 12D3KooWFrnuj5o3tx4fGD2ZVJRyDqTdzGnU3XYXmBbWbc8Hs8Nd-12D3KooWHF8UTBzeATDerVFXtZYfBdFBDz82Nn2B5uZ2m1jbejQe-1639108958367248635"
      ]
    }
  }
}

For Discussion

This is primarily to support Project Lightning and the test miner we run.
However, it maybe useful for other miners as well. That's why I'm actually PR'ing this.

BUT: while technically these methods are public on graphsync and data transfer, as you can see from the code they're public on some concrete types, and there's some forced casting here. That's cause they're not really "done" -- we expect them to change a few times over the next few months. So if we expose this in Lotus master, miners need to be aware they can expecting changing behavior over time.

@hannahhoward hannahhoward requested a review from a team as a code owner December 10, 2021 20:38
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #7759 (1078dff) into master (1bc9877) will decrease coverage by 0.10%.
The diff coverage is 6.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7759      +/-   ##
==========================================
- Coverage   39.51%   39.40%   -0.11%     
==========================================
  Files         654      654              
  Lines       70049    70170     +121     
==========================================
- Hits        27678    27651      -27     
- Misses      37615    37756     +141     
- Partials     4756     4763       +7     
Impacted Files Coverage Δ
api/api_storage.go 0.00% <ø> (ø)
api/types.go 52.77% <ø> (ø)
cmd/lotus-miner/market.go 22.17% <0.00%> (-0.89%) ⬇️
node/impl/storminer.go 22.05% <0.00%> (-5.67%) ⬇️
node/builder_miner.go 94.96% <100.00%> (+0.07%) ⬆️
node/modules/storageminer.go 63.35% <100.00%> (+0.11%) ⬆️
markets/retrievaladapter/client_blockstore.go 62.50% <0.00%> (-6.25%) ⬇️
chain/stmgr/execute.go 86.95% <0.00%> (-4.35%) ⬇️
chain/messagepool/repub.go 51.61% <0.00%> (-4.31%) ⬇️
chain/stmgr/call.go 71.51% <0.00%> (-3.64%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bc9877...1078dff. Read the comment docs.

go.mod Outdated
@@ -66,7 +66,7 @@ require (
github.com/icza/backscanner v0.0.0-20210726202459-ac2ffc679f94
github.com/influxdata/influxdb1-client v0.0.0-20200827194710-b269163b24ab
github.com/ipfs/bbloom v0.0.4
github.com/ipfs/go-bitswap v0.3.4
github.com/ipfs/go-bitswap v0.4.1-0.20211029155204-92d1e7aaf1dd
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use a tagged version

Copy link
Member

Choose a reason for hiding this comment

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

blocked by the context thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blocked on the darn context data store change -- there is no tagged version between 0.3.4 and the context change that includes the independent updates needed (tied to update go-peertaskqueue)

@jennijuju jennijuju merged commit 921fda9 into master Dec 22, 2021
@jennijuju jennijuju deleted the feat/transfer-diagnostics branch December 22, 2021 22:07
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.

3 participants