Joystream / joystream

Joystream Monorepo
http://www.joystream.org
GNU General Public License v3.0
1.42k stars 115 forks source link

Colossus: Sync/Cleanup - Do not skip fetching from all buckets operated by worker. #4990

Closed mnaamani closed 9 months ago

mnaamani commented 9 months ago

Problem

When preparing download tasks, and cleanup tasks, the node is preparing list of urls/endpoints to fetch and check availability of objects at those endpoints.

https://github.com/Joystream/joystream/blob/5d911ce1bd546da16c983c852203a2a6eac8d8ea/storage-node/src/services/sync/synchronizer.ts#L117

https://github.com/Joystream/joystream/blob/5d911ce1bd546da16c983c852203a2a6eac8d8ea/storage-node/src/services/sync/cleanupService.ts#L126

The implementation attempts to avoid using endpoints of buckets associated with the worker running the current node. Although the intent is correct: "do not try to fetch from self", it ignores the case where the worker is operating buckets on different nodes (which is recommended) and needs to fetch an object from one of their remote nodes.

Solution(s)

  1. Instead of passing the current worker id, pass in the current buckets being operated by the node and skip those buckets endpoints. or
  2. Add a new argument to the server command with the public endpoint of the node, and only that will be used to identify endpoints not to connect to.

Prevent Self-Connecting

This also brings up a subtle issue that can arise from operators that mis-configure endpoints, causing a node to connect to itself to fetch an object (eg. operator operates multiple buckets, but has only configured the current node with one bucket, so will attempt to fetch/check presence of an object from/on a remote bucket which is actually pointing to itself). This isn't really a problem for sync, but more for cleanup since it will be a false-positive counting to replication count.