storj-archived / core

Deprecated. Implementation of the Storj v2 protocol for Node.js.
https://storj.io
Other
396 stars 88 forks source link

dont trigger noSpaceLeft on huge shards #753

Closed littleskunk closed 6 years ago

littleskunk commented 6 years ago
braydonf commented 6 years ago

So without this, I do not think spaceAvailable will ever be set to false, as we're not running runSpaceCheck to start the interval that will check it due to that method currently using too much CPU and memory on large databases.

We are running the connectBridges interval that will update a bridge contact info for a farmer to have spaceAvailable if the values differ (https://github.com/littleskunk/core/blob/644d4af70317eba050e55b751fff074a7866a18e/lib/network/farmer.js#L380-L385).

littleskunk commented 6 years ago

Can you link that interval? I haven't seen something like that. The connectBridges is called only once on startup as far as I know.

braydonf commented 6 years ago

Yeah it's here: https://github.com/littleskunk/core/blob/644d4af70317eba050e55b751fff074a7866a18e/lib/network/farmer.js#L313-L319

littleskunk commented 6 years ago

How about this: https://github.com/Storj/core/blob/9691c6a17991a0487e1069a1000db5035b1c57ba/lib/network/farmer.js#L284-L286

As far as I can see the farmer is already connected with the bridge and will skip every additional call?

braydonf commented 6 years ago

Ah yeah, that could be it.

braydonf commented 6 years ago

The check for differences in contact data is after that check in _connectBridge

braydonf commented 6 years ago

I probably neglected to write an integration test for all those methods being used together, and hence the oversight.

braydonf commented 6 years ago

Okay, merging this and we can fix updating spaceAvailable in a new PR, and then we can make a release

braydonf commented 6 years ago

Actually, @littleskunk the connected flag gets set to false when noSpaceLeft is called, which should trigger the check and update: https://github.com/Storj/core/blob/master/lib/network/farmer.js#L365

braydonf commented 6 years ago

We may just need to fix runSpaceCheck and re-enable that interval.

braydonf commented 6 years ago

Continuing on this with a new PR https://github.com/Storj/core/pull/755