Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

13/WAKU2-STORE / 14/WAKU2-MESSAGE: Allow messages to be marked as "don't store" #441

Closed
oskarth opened this issue Aug 2, 2021 · 7 comments

Comments

@oskarth
Copy link
Contributor

oskarth commented Aug 2, 2021

Original problem

Sometimes the application might produce messages that don't need to be stored, an example would be a ping to show that you are online, or for example messages that are time sensitive (say a push notification, through our current push notification system, which would be ignored if retrieved at a later date).

In those cases it would be useful to mark them as "don't store", so that they are not stored by store nodes. This would improve bandwidth performance of the overall network and of the clients, at the cost of some metadata leaking (though can be optional).

Not a crucial feature, but there would be a few use cases for us.

Suggested solution (Oskar)

Good idea! Would a ephemeral field in Waku Message spec https://rfc.vac.dev/spec/14/ work? Then by convention store nodes wouldn't store those messages.

Similar to MVDS metadata bool ephemeral = 2;

Originally posted by @oskarth in #427 (comment)

Reply (cammellos)

@oskarth yes, precisely that would work, thanks!

Another option, it could be a bit more generic, say to allow a StoreTTL, after which point messages are not sent to users from mailservers, so you can still pull them if you just arrived online, and cover both cases.
Say something like:

StoreTTL=x means that if I fetch messages those that have SenderTime + TTL < RequestTimeFrom < RequestTimeTo

In this case it covers the case outlined above and also avoids the scenario of a user coming back online and just missing a useful message.

cc @richard-ramos

@oskarth oskarth changed the title 14/WAKU2-MESSAGE: Add ephemeral field 13/WAKU2-STORE / 14/WAKU2-MESSAGE: Allow messages to be marked as "don't store" Aug 2, 2021
@oskarth
Copy link
Contributor Author

oskarth commented Aug 2, 2021

Migrating from discussions (#427 will be dead soon).

Assuming this #427 (comment) clears up the other issue. If not, let's open a separate issue for that problem.

For now I'd probably err on the side of having a simple ephemeral field, unless we have see a strong use case for TTL? Considering what https://rfc.vac.dev/spec/21/ provides

@richard-ramos
Copy link
Member

richard-ramos commented Aug 6, 2021

I can see having both ephemeral fields and TTL being useful for different situations:

  1. The messages used for syncing channels/contacts can be considered ephemeral as it is assumed that all the devices being sync'd are online at the same time. It probably does not make sense to store these messages.

  2. TTL might be useful with the Online/Offline Status, for example, with the current implementation, all users broadcast their status every 5 minutes, so something like this might happen:

    • Alice logins into the app, and broadcast her status
    • Bob logins 2 minutes later, and retrieves all messages sent in the content-topic for status updates
    • If the status messages are ephemeral, then the last status of Alice will not be retrieved and Bob will have to wait until Alice broadcast her status in ~3 minutes (She'll appear offline in the meantime)
    • If the status messages use instead a TTL with a value greater than the broadcast interval of 5 min, the last status update of Alice will be retrieved, so she will appear online for Bob.

@staheri14
Copy link
Contributor

Thanks, @richard-ramos for your comment! That makes perfect sense! Do you think that is a pressing need? I mean the support for TTL and ephemeral fields?

@richard-ramos
Copy link
Member

I think these features are "Nice to have"s. The Status app can still work without the existence of these fields. We just need to be aware that messages that are supposed to be ephemeral, will be persisted on the DB which might not be ideal (due to wasted storage space allocated to these).

@jm-clius
Copy link
Contributor

Linking this issue to waku-org/nwaku#702, as this feature could go a long way in alleviating the memory/disk issues we're trying to address for store

@rymnc
Copy link
Contributor

rymnc commented Sep 7, 2022

Decided not to include storeTTL in #532, since this could be handled by applications that require it. Additionally, this increases the dependency of WakuMessage upon WakuStore (which is an optional module for waku nodes).

What do you think?
cc: @richard-ramos

@rymnc
Copy link
Contributor

rymnc commented Sep 13, 2022

Completed in #532

@rymnc rymnc closed this as completed Sep 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants