Store MLS messages separately #330
Conversation
| }, nil | ||
| } | ||
|
|
||
| func (s *Store) QueryMessages(ctx context.Context, query *messagev1.QueryRequest) (*messagev1.QueryResponse, error) { |
There was a problem hiding this comment.
The mixing of messagev1 types into this feels a bit uncomfortable
| if s.mlsStore == nil { | ||
| return nil, errors.New("mls store not configured") | ||
| } | ||
| return s.mlsStore.QueryMessages(ctx, req) |
There was a problem hiding this comment.
Alternatively we could add QueryMessages to the mls/v3 service with it's own isolated types, rather than hijacking the messagev1 service+types for it, but that would have some downstream impact
There was a problem hiding this comment.
Downstream impact isn't a huge deal right now. There isn't that much downstream at the moment. Basically a single caller.
There was a problem hiding this comment.
Ok great I think I'll add the mls/v3 protos to separate this all out cleanly then
|
Converted back into a draft PR while I do the separated proto work for querying. |
| Where("id = ?", installationId). | ||
| Where("revoked_at IS NULL"). | ||
| Exec(ctx) | ||
|
|
||
| return err | ||
| } | ||
|
|
||
| func (s *Store) InsertMessage(ctx context.Context, topic string, content []byte) (*messagev1.Envelope, error) { | ||
| createdAt := s.config.now() |
There was a problem hiding this comment.
I'm not sure this ordering is as total as we want. Clock drift between nodes could have newer messages actually end up before older messages.
|
|
||
| CREATE TABLE messages ( | ||
| topic VARCHAR(200) NOT NULL, | ||
| tid VARCHAR(96) NOT NULL, |
There was a problem hiding this comment.
Given how strict the ordering requirements are, I'd really rather this rely on a sequence than the server time. Clocks can drift, and any re-ordering can corrupt the group state with the way MLS works. Even if those conflicts are improbable, it makes debugging a lot easier if they are impossible.
There was a problem hiding this comment.
Ok agree, that makes sense. I'll update it to use a simpler sequenced id primary key.
I'm going to close down this PR as it is, I think I'd also like to just split up the messages table by type too; group vs welcome messages, and get rid of the idea of topics completely.
Fixes #319
What this includes:
What this doesn't include that the existing messages DB has: