storj-archived / core

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

Set a default max shard size at 4GiB #755

Closed braydonf closed 6 years ago

braydonf commented 6 years ago

This is the largest that is supported on the client side, furthermore there are larger issues that need to be addressed to be able to support shard sizes larger than this to reduce the number of errors.

cc: @littleskunk @tempestb @aleitner

Goes along with pull requests:

Related issues:

littleskunk commented 6 years ago

I would expect out of memory exceptions caused by usedSpace calculation. We had this problem in the past: https://github.com/Storj/storjshare-daemon/issues/247#issuecomment-330683809

I will test this with my ~4TB farmer.

braydonf commented 6 years ago

Yes, that's why I switched it from using du to diskusage.check

littleskunk commented 6 years ago

I installed your branch but running free space check is missing on my logfile. Looks like still not called.

root@storj:~# npm list -g storj-lib
/usr/local/lib
└─┬ storjshare-daemon@5.3.0
  └── storj-lib@8.5.0  (github:braydonf/core#e7a3f2b8f2ffe1fa35cee49b7b055e7f00822345)
braydonf commented 6 years ago

You may need to add farmer.runSpaceCheck() in scripts/farmer.js in storjshare-daemon.

littleskunk commented 6 years ago

Looks better now. The check is running. I don't see any memory problems.

Please take a look at: https://github.com/Storj/core/issues/738

The SpaceCheck ignores my allocated size and is only looking on the total free space of my drive. That will not fix this issue.

braydonf commented 6 years ago

spaceAvailable should be set, we can test to confirm. The issue with ignoring the allocated size will need to be addressed in another PR (as that check is currently expensive to make).

littleskunk commented 6 years ago

Yea that is correct. At the moment we only see the total used space of that drive and can't see how many of that are storj shards. That was the part with the out of memory exception we don't want to get again.

Ok #738 and https://github.com/Storj/storjshare-daemon/issues/307 can't be fix that easy.

braydonf commented 6 years ago

Yeah, they require some larger overhauling.

However immediately concerns should be addressed with https://github.com/Storj/bridge/pull/547 for https://github.com/Storj/storjshare-daemon/issues/307

aleitner commented 6 years ago

I merged models. Now bridge needs to update the package json to use that

braydonf commented 6 years ago

Models isn't part of this, but I think we're good on this one to merge.

braydonf commented 6 years ago

And then we can make a release at v8.6.0