change how we store SKF for performance gains #256
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SKF are stored with the timestamp of the last time they were used to make it more likely to hit a session key for a packet early. An ets ordered_set table was used to achieve this, but it follows total sorting, which means the timestamp needs to be the first element in the table if we want that to take precedence. Part of removing, or updating, a SKF involved `ets:select_delete/2` which requires a full table scan because we only had the SessionKey at the time of deletion. When maybe removes or updates come in from the config service, all of those updates would be spawned into their own process and contend for the SKF table at the same time. For sufficiently large ets tables (+10k) and sufficiently large batches of updates (1k), this would cause all schedulers to have trouble doing anything while tables scans were happening. This pausing would mean the grpc connection could not talk back to the config-service and negotiate a slower rate of updates, causing them to get out of order, and grpcbox would kill the connection. And it would all start again. The structure of the table has been flattened, so we can tell ets the key position is the SessionKey in the second slot. It will still respect total ordering with the timestamp in the first position, but we can now do deletes in constant time with only the SessionKey. 💃
|
The test data I was using mislead me. It also sorts on keypos. :sigh: |
macpie
reviewed
Sep 5, 2023
| 0 -> lager:debug(MD, "inserted SKF"); | ||
| _ -> lager:debug([{deleted, Deleted} | MD], "updated SKF") | ||
| end; | ||
| true = do_remove_skf(SKFETS, SKF), |
There was a problem hiding this comment.
Can we make them return ok for both?
macpie
reviewed
Sep 5, 2023
| ordered_set, | ||
| {read_concurrency, true}, | ||
| {write_concurrency, true}, | ||
| {keypos, 2}, |
There was a problem hiding this comment.
idk if this will work as the ordered_set will be on the key. Can you 2x check?
There was a problem hiding this comment.
one key, one object, ordered in Erlang term order
Maybe it's fine
macpie
reviewed
Sep 5, 2023
| @@ -572,7 +570,10 @@ do_insert_skf(SKFETS, SKF) -> | |||
| %% This is negative to make newest time on top | |||
| Now = erlang:system_time(millisecond) * -1, | |||
| true = ets:insert(SKFETS, { | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SKF are stored with the timestamp of the last time they were used to make it more likely to hit a session key for a packet early. An ets ordered_set table was used to achieve this, but it follows total sorting, which means the timestamp needs to be the first element in the table if we want that to take precedence.
Part of removing, or updating, a SKF involved
ets:select_delete/2which requires a full table scan because we only had the SessionKey at the time of deletion. When maybe removes or updates come in from the config service, all of those updates would be spawned into their own process and contend for the SKF table at the same time. For sufficiently large ets tables (+10k) and sufficiently large batches of updates (1k), this would cause all schedulers to have trouble doing anything while tables scans were happening. This pausing would mean the grpc connection could not talk back to the config-service and negotiate a slower rate of updates, causing them to get out of order, and grpcbox would kill the connection. And it would all start again.The structure of the table has been flattened, so we can tell ets the key position is the SessionKey in the second slot. It will still respect total ordering with the timestamp in the first position, but we can now do deletes in constant time with only the SessionKey. 💃