skip url endpoints of buckets operated by the node

Joystream / joystream

Joystream Monorepo

http://www.joystream.org

GNU General Public License v3.0

1.42k stars 115 forks source link

skip url endpoints of buckets operated by the node #4993

Closed mnaamani closed 10 months ago

mnaamani commented 11 months ago

Fixes https://github.com/Joystream/joystream/issues/4990

When selecting "candidate" urls to fetch from, instead of ignoring all endpoints of buckets operated by worker id, we select all urls of buckets except the ones operated by the node. This ensures we don't miss any remote urls of buckets (even if they are operated by the same worker) on a different host.
Just in-case there is a misconfiguration by the operator with respect to url endpoints, and to avoid the node connecting to itself, we send an http header X-COLOSSUS-HOST-ID with a new random value when the node starts up, which is checked by express server when handling requests. If the value belongs to the node, the request is dropped.

mnaamani commented 10 months ago

Instead of doing this, shouldn't we consult QN i.e. if node operator didn't specify any bucket/s with the current node, and storageBucket.operatorMetadata.nodeEndpoint of that bucket/s is equal to the endpoint 1 of the current running node, then we just ignore it.

That a good suggestion. I added this check in a3cb8e5

I would argue, there is still value in using the header as well to catch the case where two url endpoints might be different (by string comparison), but still actually point to the same host (ie DNS records point to same host, irrespective if intentional or accidental)

mnaamani commented 10 months ago

Just a note on your comment "if node operator didn't specify any bucket/s with the current node" - if no buckets are specified as command line arguments, then all buckets the worker is associated with are selected.

Regarding "add a condition upon storage-node bootstrapping that all the buckets specified should have the same endpoint". Are you suggesting the node should exit with failure if that is not the case or just log a warning? It could be useful to do an info log so the operator is aware what the metadata endpoint url is for each bucket they are serving.

zeeshanakram3 commented 10 months ago

Regarding "add a condition upon storage-node bootstrapping that all the buckets specified should have the same endpoint". Are you suggesting the node should exit with failure if that is not the case or just log a warning? It could be useful to do an info log so the operator is aware what the metadata endpoint url is for each bucket they are serving.

I was suggesting exiting the node with failure, but since as you pointed out two different endpoints can resolve to the same host, I think showing a warning log would be ideal.