Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Strange "not subscribed to pubsubTopic" errors for filter/v2/messages GET requests #960

Closed
fbarbu15 opened this issue Dec 13, 2023 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@fbarbu15
Copy link

Steps to reproduce the behavior:

  1. Start node1(gowaku) on filter and relay protocols (have min_relay_peers_to_publish = 0 for simplicity)
  2. Start node2(gowaku) on filter protocol
  3. Connect/dial node2 to node1
  4. node1 subscribes via relay to "pubsubTopic": "/waku/2/rs/18/1"
  5. node2 subscribes via filter to "contentFilters": ["/test/1/waku-filter/proto"], "pubsubTopic": "/waku/2/rs/18/1"
  6. node1 publishes a message via relay on the above pubsubtopic that matches the above content topic: {"payload": "RmlsdGVyIHdvcmtzISE=", "contentTopic": "/test/1/waku-filter/proto", "timestamp": 1702470394471216640}
  7. node2 fetched new messages on the above content topic:
    => Very strange error: 'not subscribed to pubsubTopic:/waku/2/rs/1/4 contentTopic: /test/1/waku-filter/proto'
    Strange because /waku/2/rs/1/4 is not used anywhere

Here's the test logs:

DEBUG    src.node.api_clients.base_client:base_client.py:11 POST call: http://127.0.0.1:1854/relay/v1/subscriptions with payload: ["/waku/2/rs/18/1"]
INFO     src.node.api_clients.base_client:base_client.py:22 Response status code: 200. Response content: b'true'
DEBUG    src.node.api_clients.base_client:base_client.py:11 POST call: http://127.0.0.1:26265/filter/v2/subscriptions with payload: {"requestId": "6b54b7d3-162b-4054-b173-fa638d4c76d4", "contentFilters": ["/test/1/waku-filter/proto"], "pubsubTopic": "/waku/2/rs/18/1"}
INFO     src.node.api_clients.base_client:base_client.py:22 Response status code: 200. Response content: b'{"requestId":"6b54b7d3-162b-4054-b173-fa638d4c76d4","statusDesc":"OK"}'
DEBUG    src.node.api_clients.base_client:base_client.py:11 POST call: http://127.0.0.1:1854/relay/v1/messages/%2Fwaku%2F2%2Frs%2F18%2F1 with payload: {"payload": "RmlsdGVyIHdvcmtzISE=", "contentTopic": "/test/1/waku-filter/proto", "timestamp": 1702470394471216640}
INFO     src.node.api_clients.base_client:base_client.py:22 Response status code: 200. Response content: b'true'
DEBUG    src.libs.common:common.py:33 Sleeping for 0.1 seconds
DEBUG    src.steps.filter:filter.py:87 Checking that peer NODE_1:harbor.status.im/wakuorg/go-waku:latest can find the published message
DEBUG    src.node.api_clients.base_client:base_client.py:11 GET call: http://127.0.0.1:26265/filter/v2/messages/%2Ftest%2F1%2Fwaku-filter%2Fproto with payload: None
ERROR    src.node.api_clients.base_client:base_client.py:16 HTTP error occurred: 404 Client Error: Not Found for url: http://127.0.0.1:26265/filter/v2/messages/%2Ftest%2F1%2Fwaku-filter%2Fproto. Response content: b'not subscribed to pubsubTopic:/waku/2/rs/1/4 contentTopic: /test/1/waku-filter/proto'

go-waku version/commit hash

harbor.status.im/wakuorg/go-waku:latest

Additional context
If I use harbor.status.im/wakuorg/nwaku:latest instead of go-waku the test passes.

Here's the docker log
node2_2023-12-13_14-26-09__2e5a7b17-7a3a-4811-b3a6-11406583d9a1__harbor.status.im_wakuorg_go-wakulatest.log
node1_2023-12-13_14-26-09__2e5a7b17-7a3a-4811-b3a6-11406583d9a1__harbor.status.im_wakuorg_go-wakulatest.log

@fbarbu15 fbarbu15 added the bug Something isn't working label Dec 13, 2023
@richard-ramos richard-ramos self-assigned this Dec 13, 2023
@richard-ramos
Copy link
Member

richard-ramos commented Dec 13, 2023

Looking at the URL used to obtain the messages: http://127.0.0.1:26265/filter/v2/messages/%2Ftest%2F1%2Fwaku-filter%2Fproto, I see that pubsub topic was calculated automatically because it assumed it was using autosharding.

go-waku has the following endpoints for obtaining messages in filter:

  • GET /filter/v2/messages/{contentTopic} - Meant to be used with autosharding. It will automagically calculate the pubsub topic based on the content topic
  • GET /filter/v2/messages/{pubsubTopic}/{contentTopic} - This is meant to be used with named/static sharding. It will retrieve messages from an specific pubsub topic that match a content topic.

In this regard go-waku and nwaku differ in how the REST API works. If we wanted to make go-waku match nwaku behavior, the solution is as simple as:

  • modifying
    func (c *filterCache) getMessages(pubsubTopic string, contentTopic string) ([]*RestWakuMessage, error) {
    c.mu.RLock()
    defer c.mu.RUnlock()
    if c.data[pubsubTopic] == nil || c.data[pubsubTopic][contentTopic] == nil {
    return nil, fmt.Errorf("not subscribed to pubsubTopic:%s contentTopic: %s", pubsubTopic, contentTopic)
    }
    msgs := c.data[pubsubTopic][contentTopic]
    c.data[pubsubTopic][contentTopic] = []*RestWakuMessage{}
    return msgs, nil
    }
    so we only take into account the content topic and not the pubsub topic.
  • Removing the GET /filter/v2/messages/{pubsubTopic}/{contentTopic} handler.

What do you think, @chaitanyaprem?

/

@chaitanyaprem
Copy link
Collaborator

chaitanyaprem commented Dec 13, 2023

Looking at the URL used to obtain the messages: http://127.0.0.1:26265/filter/v2/messages/%2Ftest%2F1%2Fwaku-filter%2Fproto, I see that pubsub topic was calculated automatically because it assumed it was using autosharding.

go-waku has the following endpoints for obtaining messages in filter:

* `GET /filter/v2/messages/{contentTopic}` - Meant to be used with autosharding. It will automagically calculate the pubsub topic based on the content topic

* `GET /filter/v2/messages/{pubsubTopic}/{contentTopic}` - This is meant to be used with named/static sharding. It will retrieve messages from an specific pubsub topic that match a content topic.

In this regard go-waku and nwaku differ in how the REST API works. If we wanted to make go-waku match nwaku behavior, the solution is as simple as:

* modifying https://github.com/waku-org/go-waku/blob/c403388ec2146eb93155ee47f92f8eb030190857/cmd/waku/server/rest/filter_cache.go#L74-L84
   so we only take into account the content topic and not the pubsub topic.

* Removing the `GET /filter/v2/messages/{pubsubTopic}/{contentTopic}` handler.

What do you think, @chaitanyaprem?

/

I think the REST API should be updated to supported both types, because waku code/node can be used for static sharding scenarios like Status or TheGraph and autosharding is used in case of Waku Network. It would be better to have both kinds of API's rather than only autosharding.

@chaitanyaprem
Copy link
Collaborator

  • GET /filter/v2/messages/{contentTopic} - Meant to be used with autosharding. It will automagically calculate the pubsub topic based on the content topic

  • GET /filter/v2/messages/{pubsubTopic}/{contentTopic} - This is meant to be used with named/static sharding. It will retrieve messages from an specific pubsub topic that match a content topic.

@NagyZoltanPeter nwaku filter REST API also should be updated to support both static and autosharding as indicated here. Is this already being tracked as an issue?

@NagyZoltanPeter
Copy link

  • GET /filter/v2/messages/{contentTopic} - Meant to be used with autosharding. It will automagically calculate the pubsub topic based on the content topic
  • GET /filter/v2/messages/{pubsubTopic}/{contentTopic} - This is meant to be used with named/static sharding. It will retrieve messages from an specific pubsub topic that match a content topic.

@NagyZoltanPeter nwaku filter REST API also should be updated to support both static and autosharding as indicated here. Is this already being tracked as an issue?

I had to take a closer look in nwaku, but still I have doubts on it.
Static sharding is still possible in nwaku without having to add such an API.

As being handled while registering the subscriptions, it is handled by having a non empty pubsub topic specified in the subscription... than it is static sharding otherwise auto shard is calculated and subscription is registered for that way.

https://github.com/waku-org/nwaku/blob/master/waku/node/waku_node.nim#L547

On the getMessage side in either case you must only specify content topic to get the relevant messages, never the pubsubTopic.
Am I miss something? How is the API is being used by those client implementations?

@SionoiS WDYT?

@SionoiS
Copy link

SionoiS commented Jan 3, 2024

Couple of points;

  • Autosharding cannot be used when cluster id is not equal to 1. Autosharding is ONLY defined in the context of TWN.
  • Nwaku cannot be used to get msg on pubsub topic. We could add the same endpoint as go-waku.
  • FILTER is a PUSH protocol. getMessage is only a convenience for node operators.

Knowing this, I would only change the test to cluster id 1 and not change anything else.

@NagyZoltanPeter
Copy link

NagyZoltanPeter commented Jan 3, 2024

Couple of points;

  • Autosharding cannot be used when cluster id is not equal to 1. Autosharding is ONLY defined in the context of TWN.
  • Nwaku cannot be used to get msg on pubsub topic. We could add the same endpoint as go-waku.

When using getMessage REST API, you should use it in context of your subscriptions, so I see no point to add an explicit path param of pubsubTopic for getMessage.

  • FILTER is a PUSH protocol. getMessage is only a convenience for node operators.

Well among nodes it is a push protocol definitely but from the client perspective, I though it is a doable config to run up a node for your app and use it through the API. Hence my question about the use case of Status app and/or TheGraph!

Knowing this, I would only change the test to cluster id 1 and not change anything else.

@SionoiS
Copy link

SionoiS commented Jan 3, 2024

Well among nodes it is a push protocol definitely but from the client perspective, I though it is a doable config to run up a node for your app and use it through the API. Hence my question about the use case of Status app and/or TheGraph!

Knowing this, I would only change the test to cluster id 1 and not change anything else.

Yes your right, NVM. We have to change this, it's not very ergonomic.

@chaitanyaprem
Copy link
Collaborator

Autosharding cannot be used when cluster id is not equal to 1. Autosharding is ONLY defined in the context of TWN.

Interesting, I am wondering if this also can be provided as a feature in case someone wants to use static-sharded network.
Because all someone like status has to do is just indicate how many shards they want in the network and same algo can be applied right. Do we see some limitation where autosharding is bound by clusterID 1? I maybe missing something though.

@chair28980
Copy link
Contributor

@waku-org/nwaku-developers please create a ticket in the nwaku repo to update the REST api on the nwaku side per the above discussion.

@NagyZoltanPeter
Copy link

NagyZoltanPeter commented Jan 17, 2024

@chaitanyaprem, @SionoiS , CC: @chair28980, @waku-org/nwaku-developers:

I still feel the endpoing GET /filter/v2/messages/{pubsubTopic}/{contentTopic} missleading and overkill.
Client in either case will look for messages under certain content-topic, regardless of sharding type.
A client can have certain subscriptions, do we expect that under different pubsub-topic the content-topics will be duplicates?
On the other hand, such a resource indexing .../messages/{pubsubTopic}/{contentTopic} is avoidable due to error handling reasons as on server side cannot be decided that a client missuse the API by not providing contentTopic after pubsubTopic. In this sense it should rather be path named arguments than path segments.
Btw we also had a discussion recently to rather use POST request with json in BODY than have to URL encode pubsub and content-topics.

Please 'convince' me to introduce such an endpoint "GET /filter/v2/messages/{pubsubTopic}/{contentTopic}".
I mean I miss the use case for it from the above discussion.
@SionoiS: Are we sure about that the message cache we use handle properly such situaiton among different subscriptions? I mean does it works as a classic pub-sub queue as topic type (sorry industry namings can be confusing in our terms)? I see we expect one filterclient node is expected to be used by only one thin client over REST.
Like: web-app - REST - filterclient node - filter service node - NETWORK...
[EDIT] Yes this last one question maybe cumbersome as even though it is not stated with current REST interface that is the only one logical usage, where a thin client uniquely uses a single filter-client node to communicate over the network.

@SionoiS
Copy link

SionoiS commented Jan 17, 2024

@SionoiS: Are we sure about that the message cache we use handle properly such situaiton among different subscriptions? I mean does it works as a classic pub-sub queue as topic type (sorry industry namings can be confusing in our terms)?

The message cache works as follow; you can get a message by either content topic OR pubsub topic and it is removed only if too old or unsubscribed from both content and pubsub topics.

@chaitanyaprem
Copy link
Collaborator

Client in either case will look for messages under certain content-topic, regardless of sharding type.

If a client is using a static sharded network, the client would have to specify both pubsub and contentTopic in order to retrieve a message isn't it? Or is the messageCache global and not specific to a pubsubTopic?

Because in go-waku the messageCache is for specific contentFilter (which can be either only pubsubTopic or pubsub and list of contentTopics) which means a query to fetch messages can be made either with:

  • contentTopic only (which would use autosharding algo to determine the pubsubTopic and in-turn find a contentFilter match and lookup in specific messageCache)
  • pubsub and contentTopic (which would look for messageCache based on contentFilter based on this)

Is the messageCache a single queue for all subscribed pubsubTopics and inturn contentTopics?

@chaitanyaprem
Copy link
Collaborator

Also note that In case of static-sharding, same contentTopic can be present or used in multiple pubsubTopics . i.e C1 contentTopic can be used in P1 and P2 which are different pubsubTopics/shards.
I know this is a corner-case, but it is possible.

So, without having an ability of specifing pubsubTopic how can a user retrieve messages for a contentTopic C1 under P1 via the REST API?

In case of autosharding, contentTopic automatically maps to a pubsubTopic which would avoid this situation.

@SionoiS
Copy link

SionoiS commented Jan 25, 2024

Is the messageCache a single queue for all subscribed pubsubTopics and inturn contentTopics?

It is per protocol, e.g. one cache for relay one for filter.

Also note that In case of static-sharding, same contentTopic can be present or used in multiple pubsubTopics . i.e C1 contentTopic can be used in P1 and P2 which are different pubsubTopics/shards.
I know this is a corner-case, but it is possible.

Not possible in Nwaku AFAIK

So, without having an ability of specifing pubsubTopic how can a user retrieve messages for a contentTopic C1 under P1 via the REST API?

ATM, they can't because content topic are always autosharded.

They can get ALL the msgs from a pubsub topic and filter at the app lvl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

6 participants