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

chore: extract daemon & update to go 1.19 #33

Merged
merged 9 commits into from
May 18, 2023

Conversation

laurentsenta
Copy link
Contributor

No description provided.

@laurentsenta
Copy link
Contributor Author

laurentsenta commented Feb 28, 2023

A quick test on my machine gave the same result between this version and https://ipfs-check.on.fleek.co.

Marking as draft until I resolve this warning:

2023-02-28T11:35:57.280+0100 ERROR fullrtdht fullrt/dht.go:309 Accelerated DHT client was unable to fully refresh its routing table due to Resource Manager limits, which may degrade content routing. Consider increasing resource limits. See debug logs for the "dht-crawler" subsystem for details.
Ready to start serving

Related + ideas for fixes:
libp2p/go-libp2p-kad-dht#806, ipfs/kubo#9407, ipfs/kubo#9405, ipfs/kubo#8945

@laurentsenta laurentsenta marked this pull request as draft February 28, 2023 10:47
@laurentsenta
Copy link
Contributor Author

@SgtPooki fyi that PR would fix #31

@laurentsenta
Copy link
Contributor Author

PR fixed with infinite resources,

In debug logging only, there are a few:

2023-03-03T11:04:42.088+0100 DEBUG dht-crawler crawler/crawler.go:239 error finding data on peer 12D3KooWHjFZWS5hrG67tguoQSSPDn2aisWv7SPdggxYDPdwfpA8 with cpl 0 : Application error 0x0 (remote): conn-2139226: system: cannot reserve inbound connection: resource limit exceeded

but the dht-crawler is reaching out 25k connections without too much trouble.

@laurentsenta laurentsenta marked this pull request as ready for review March 3, 2023 10:13
@laurentsenta laurentsenta marked this pull request as draft March 3, 2023 10:28
Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

Haven't looked super carefully here, but looks like some cut/paste cleanup and some much needed dependency upgrades.

Have you run this (either locally or on the check.ipfs.network backend) to make sure it's performing alright?

@laurentsenta
Copy link
Contributor Author

laurentsenta commented May 5, 2023

@aschmahmann thanks for the review!

looks like some cut/paste cleanup and some much needed dependency upgrades.

Correct. This is preparatory work for more refactor (adding sentry, some API framework, etc)

Have you run this (either locally or on the check.ipfs.network backend) to make sure it's performing alright?

I did, and it seems to work OK, but as we discussed on Slack I'm seeing shakiness in prod already. For example, the empty dir is not found in the DHT as I write this. So it's hard to assert what's new or old flakiness.

I'll include some testing to make sure we improve the situation.

@laurentsenta laurentsenta force-pushed the prepare-upgrade branch 2 times, most recently from 70a2b32 to 531051b Compare May 15, 2023 12:51
obj.Value("DataAvailableOverBitswap").Object().Value("Responded").Boolean().IsTrue()
},
"ipfs", "dht", "provide", randomFileCid, "--verbose")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disabled for now

// obj.Value("DataAvailableOverBitswap").Object().Value("Error").String().IsEmpty()
// obj.Value("DataAvailableOverBitswap").Object().Value("Found").Boolean().IsTrue()
// obj.Value("DataAvailableOverBitswap").Object().Value("Responded").Boolean().IsTrue()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly disabled for now

@laurentsenta
Copy link
Contributor Author

@aschmahmann I added a few tests, example run.

@laurentsenta laurentsenta merged commit cf5b6c0 into ipfs:main May 18, 2023
@laurentsenta laurentsenta deleted the prepare-upgrade branch May 18, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants