search

waku-org / nwaku

Waku node and protocol.
Other
198 stars 51 forks source link

bug?: autosharding getting enabled for clusters != 1 #2898

Closed richard-ramos closed 1 month ago

richard-ramos commented 1 month ago

Problem

This might not be a bug but intended behavior, but just in case: While testing interop between go-waku and nwaku for storev3, I noticed the following:

  1. I started nwaku with the following flags: 
    --relay --store --nodekey=1122334455667788990011223344556677889900112233445566778899001122 --cluster-id=99
  2. The logs displayed the following: 
    nwaku-1  | INF 2024-07-11 14:19:59.661+00:00 Created WakuMetadata protocol              topics="waku node" tid=1 file=protocol.nim:117 clusterId=99 shards="{3, 4, 2, 6, 5, 7, 0, 1}"
nwaku-1  | INF 2024-07-11 14:19:59.661+00:00 mounting sharding                          topics="waku node" tid=1 file=waku_node.nim:215 clusterId=99 shardCount=0

    (Notice that shards 0 to 7 are being added automatically.

Please clarify whether this is the intended behavior. In go-waku the tests use content topics with random format, so assuming autosharding is getting enabled by default for all clusterIDs, maybe changes must be done to ensure we use the correct format?, as well as ensure the format for the content topics in status-go is also supported

fryorcraken commented 1 month ago

cc @gabrielmer who is doing some refactoring around that.

I am also aiming to improve this logic with https://github.com/waku-org/nwaku/pull/2859

chaitanyaprem commented 1 month ago

I am wondering if it is happening because of default config if shards are not specified

https://github.com/waku-org/nwaku/blob/1c9eb274155311c8cc6ae3b449439421d1dc4b6f/waku/factory/external_config.nim#L312-L326

gabrielmer commented 1 month ago

We did enable autosharding for any cluster in https://github.com/waku-org/nwaku/pull/2505 The way it works is by activating it with the amount of shards passed with the --pubsub-topic parameter

The idea is in my PR deprecating the --pubsub-topic parameter, introduce a new flag named something like --network-shards where users in clusters different than TWN set how many shards are in the network and that number will be used when running autosharding's algorithm

gabrielmer commented 1 month ago

@richard-ramos WDYT? given that autosharding is enabled by default in every cluster, is there any bug in here? not sure if to whether to close this issue or if there's some action item

richard-ramos commented 1 month ago

Let's close this. I modified go-waku so it mounts metadata protocol regardless of clusterID being 0 to match nwaku's behavior.