-
Notifications
You must be signed in to change notification settings - Fork 1.1k
statement-store: reduce index memory footprint with about 200 bytes #10894
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandru Gheorghe <[email protected]>
| if let Some((topics, key)) = self.topics_and_keys.remove(hash) { | ||
| for t in topics.into_iter().flatten() { | ||
| // Read statement from database to get topics and decryption key | ||
| let statement = db |
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 don't like the idea of operating with the DB inside index?
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.
What is your concern here ?
Signed-off-by: Alexandru Gheorghe <[email protected]>
bkchr
left a comment
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.
So you are trading memory footprint for IO? :D
| // Fallback: iterate through all indexes if database read failed | ||
| log::warn!( | ||
| target: LOG_TARGET, | ||
| "Failed to read statement {:?} from database, falling back to full index scan", |
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.
Where is this reading from database? Also can we make this debug?
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 see where it is reading from db :D
| } | ||
| } else { | ||
| // Fallback: iterate through all indexes if database read failed | ||
| log::warn!( |
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.
| log::warn!( | |
| log::debug!( |
Yes, that's my intention with this PR, with the caveat that expiring statements is not something we do very often and it is not time sensitive. |
The
topics_and_keysis eating about 200 bytes per statement, which is about 30% of the contribution for each statement.Since this structure was need only when we clean up of statement it makes more sense to just read the statement from the database instead of paying such a high memory price for each statement.