-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat: Add watcher support indexing multiple homeservers #314
base: main
Are you sure you want to change the base?
Conversation
@@ -309,3 +310,20 @@ pub fn create_file(file: &FileDetails) -> Result<Query, DynError> { | |||
|
|||
Ok(query) | |||
} | |||
|
|||
// Create a Homeserver node | |||
pub fn create_homeserver(homeserver: &Homeserver) -> Result<Query, DynError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about storing Homeserver into Neo4J also when I implemented it. I think the main issue with of storing Homeserver
in the graph is a possible dataloss of Redis indexes on crash.
In case of restart/crash, we might restore the cursor
from graph instead of Redis. Redis last snapshot might not be the latest cursor on graph, therefore, we will resume watching homeservers and skip some events.
I'm sure we can mitigate this easy by only restoring Homeserver
from Redis on watcher startup, without fallback to graph (or only on some conditions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm...I don't know if I understood that correctly or not, but I'm also updating the cursor in graph?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and that's exactly the problem: the cursor in graph might be newer than the latest synced events in Redis in case of a restart/crash.
In other words, our assumption has always been that using Redis without persistence is OK and safe if and only if when we lose Redis data we also lose the cursor. This way, in the next startup, the lost events will be re-acquired. However, saving the cursor to graph breaks this guarantee.
b2b7d7c
to
37b8c07
Compare
Fixes #200 and #209
Pre-submission Checklist
cargo nextest run
.cargo bench