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

Add ability to store message IDs persistent #541

Open
patcon opened this issue Oct 26, 2018 · 24 comments · May be fixed by #1996
Open

Add ability to store message IDs persistent #541

patcon opened this issue Oct 26, 2018 · 24 comments · May be fixed by #1996
Labels
enhancement New feature or request

Comments

@patcon
Copy link
Contributor

patcon commented Oct 26, 2018

So that between Slack instance, we can preserve threading (#529) and send emojis (#526) with more consistent behaviour :)

Thoughts? I could be pretty simple, and totally optional for people who choose to stay utterly stateless.
With matterbridge-heroku, adding persistence of relationships could be as simple as:

$ heroku addons:create heroku-redis:hobby-dev

Curious your thoughts on this very Slack-specific addition

@patcon patcon changed the title Add ability to store message IDs in option redis queue Add ability to store message IDs in optional redis queue Oct 26, 2018
@42wim 42wim added the enhancement New feature or request label Oct 26, 2018
@42wim
Copy link
Owner

42wim commented Oct 26, 2018

Quick thoughts, redis seems like overkill. (and isn't really made for persistency afaik)
I'm more for using something like boltdb (https://github.com/etcd-io/bbolt) (simple embedded database).

As long as it's optional (and simple) it's ok for me.

@patcon
Copy link
Contributor Author

patcon commented Oct 26, 2018

Bolt seems cool but I'd really prefer something that continues to work easily on heroku, where I have a few instances running :)

I'll think on it. I think you're right about redis not being for persistence. Perhaps postgres. Free plan has 10,000 rows, which exactly matches how much history our slack instance has anyhow...!

@42wim
Copy link
Owner

42wim commented Oct 26, 2018

Why do you think bolt wouldn't work?

@patcon
Copy link
Contributor Author

patcon commented Oct 27, 2018

Unless I'm misunderstanding, no heroku service addons offer bolt. Heroku is stateless without a file system, so it needs an add-on provider for its ability to have services (databases, caches, etc) as push-button

@patcon
Copy link
Contributor Author

patcon commented Oct 27, 2018

Ok, so on further investigation, redis actually seems a step in the right direction. I don't think we need full persistency perhaps, but we need to be able to query based on more than primary key (original message timestamp), which is only what the lru cache allows.

So assume an example of wanting to rethread messages. We instead need something that would allow us to query for a mID value in BrMsgID from the lru cache (using a ThreadTimestamp from thread reply ), and get the key (which is the original Timestamp). We could then know all the other messages to update from there.

For now, I suppose I could just skirt the change by looping through the lru cache. Curious your thoughts on a larger solution though

@42wim
Copy link
Owner

42wim commented Nov 24, 2018

We should use https://github.com/abronan/valkeyrie if this gets implemented.

@42wim 42wim changed the title Add ability to store message IDs in optional redis queue Add ability to store message IDs persistent Nov 24, 2018
@patcon
Copy link
Contributor Author

patcon commented Nov 25, 2018

perrrrfect. I'll prob work on this next. Getting to crunch time on another project, so might not be as soon as I'd like

@patcon
Copy link
Contributor Author

patcon commented Nov 26, 2018

Potentially helpful link: https://medium.com/@nevio/proper-way-how-to-marshal-and-unmarshal-golang-structs-within-the-redis-cache-855423902787

Was curious about using msgpack (which redis-cache golang lib uses), but seems marshalling as json should work fine: https://www.reddit.com/r/golang/comments/476fdi/what_is_best_serialization_format_for_boltdb_for/

@42wim
Copy link
Owner

42wim commented Nov 26, 2018

The valkeyrie library should handle all that. There shouldn't be any redis or bolt specific code in matterbridge.

@patcon
Copy link
Contributor Author

patcon commented Nov 26, 2018

No, valkeyrie doesn't do marshalling -- just accepts byte slices as far as I can tell. Anyhow, I have a code spike here:
https://github.com/42wim/matterbridge/compare/master...patcon:valkeyrie-persistence?expand=1

Busy week coming up, so that's likely all I'll get done, but anyone is welcome to squash my commits and then fix it up :)

EDIT: Works for edits and threads, though file cache is still using a more rudimentary connection approach from earlier in my experimentation

@42wim
Copy link
Owner

42wim commented Nov 26, 2018

@patcon okay nice!
Love your enthusiasm, but before adding any new PR's could you finish the ones that are already opened? :)

@nylen
Copy link
Contributor

nylen commented Jan 4, 2019

+1 for this feature, and also for the ability to use a local/embeddable database that is created automatically when needed. Not requiring any other services to be running on the machine is a big advantage of this program.

@sas1024
Copy link
Contributor

sas1024 commented Mar 25, 2022

May be try to use gokv - Simple key-value store abstraction and implementations for Go (Redis, Consul, etcd, bbolt, BadgerDB, LevelDB, Memcached, DynamoDB, S3, PostgreSQL, MongoDB, CockroachDB and many more)
?

@sas1024
Copy link
Contributor

sas1024 commented Apr 12, 2022

@patcon hello! Can you share your code with valkeyrie implementation for persistent message store? I want to implement this thing and any help will be useful :)

@sas1024
Copy link
Contributor

sas1024 commented Apr 14, 2022

@42wim hello! Can you share your thoughts on implementing this feature? I want to realize this feature.

For now in code i see:

  • HashiCorp LRU cache usage.
  • This cache initialized and used in multiple places (in gateway, slack, discord, mattermost, rocketchat packages)

IMHO there is two possible implementations for persistent storing message IDs:

  1. Completely replace LRU cache with valkeyrie.
  2. Use LRU cache and valkeyrie together. If no caching backend was configured, then will be used LRU cache. In this implementation we need to create abstraction between LRU cache and valkeyrie.

And we can initialize cache once and use it in all required places.

@sas1024
Copy link
Contributor

sas1024 commented Apr 20, 2022

@42wim ping? :)

@patcon
Copy link
Contributor Author

patcon commented Apr 25, 2022

A-ha! I realized that I transferred the repo to another org, and my personal fork was a more recent one without my old branches. The code is really old, but it's here 🎉
https://github.com/42wim/matterbridge/compare/master...patcon:valkeyrie-persistence?expand=1
https://github.com/g0v-network/matterbridge/tree/valkeyrie-persistence (merge conflict now, so not a clean diff, but still can check it out)

Anyhow, good luck and godspeed @sas1024 ! I'm rooting for you!

@celine-m-s
Copy link

Hey there! Any updates on this? I just implemented Matterbridge to synchronize messages between Slack and Mattermost but it tends to crash twice a week. This is not a problem in itself, except it breaks all threads (which is quite annoying) so I'd be very happy if persistent threads were in the roadmap.
By the way, thanks for all the hard work! 👏

@yousefmansy1
Copy link
Contributor

@sas1024 did you ever get anywhere on writing a solution for this feature?
I'd be willing to trying my hand at it as well.

The code link @patcon posted does not appear to exist anymore either, hoping you have it saved somewhere?

https://github.com/42wim/matterbridge/compare/master...patcon:valkeyrie-persistence?expand=1

@sas1024
Copy link
Contributor

sas1024 commented Feb 20, 2023

@yousefmansy1 I'm sorry, but I haven't started working on this feature yet :(
It well be fantastic, if you will try to realize this feature.

@yousefmansy1
Copy link
Contributor

Ok I see, no worries.
Do you have the code @patcon posted in #541 (comment)?

It seems to be a dead link for me.
image

@sas1024
Copy link
Contributor

sas1024 commented Feb 20, 2023

No, I don't have this code, this link was broken several months.
May be @patcon can share this code again?

@yousefmansy1
Copy link
Contributor

No worries, thanks for the response.
I'll probably try to take a look using the gokv thing you mentioned.

yousefmansy1 added a commit to yousefmansy1/matterbridge that referenced this issue Mar 1, 2023
@yousefmansy1 yousefmansy1 linked a pull request Mar 1, 2023 that will close this issue
yousefmansy1 added a commit to yousefmansy1/matterbridge that referenced this issue Mar 1, 2023
yousefmansy1 added a commit to yousefmansy1/matterbridge that referenced this issue Mar 14, 2023
yousefmansy1 added a commit to yousefmansy1/matterbridge that referenced this issue Mar 15, 2023
@patcon
Copy link
Contributor Author

patcon commented Sep 6, 2023

oy... i'm sorry, i'm seeing that I must have confused to clipboard, and shared the same link again. It can't be merged anymore without conflict resolution, but here's the branch with the work:
https://github.com/g0v-network/matterbridge/tree/valkeyrie-persistence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants