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

feat(projectconfig): Decompress redis value [INGEST-1505 INGEST-1506] #1345

Merged
merged 7 commits into from
Jul 21, 2022

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented Jul 21, 2022

With transaction metrics and dynamic sampling config, the size of the project config values written to redis become a problem at scale. Compression should mitigate this problem.

fn parse_redis_response(raw_response: &[u8]) -> Result<ProjectState, RedisProjectError> {
let decoded = metric!(timer(RelayTimers::ProjectStateDecompression), {
zstd::decode_all(raw_response)
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking for zstandard's magic number explicitly, I just run decompression here, assuming that the library already efficiently checks if the given blob is in zstandard format. We can use the logged metric to verify this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense. I wonder whether zstd::decode_all will allocate a vector in case the payload is not zstd. but i think either way it'll work

@jjbayer jjbayer marked this pull request as ready for review July 21, 2022 13:23
@jjbayer jjbayer requested a review from a team July 21, 2022 13:23
});

let raw_response: Cow<[u8]> = match decoded {
Ok(decoded) => decoded.into(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you just borrow here to avoid the Cow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, thanks.

@jjbayer jjbayer merged commit 0576be7 into master Jul 21, 2022
@jjbayer jjbayer deleted the feat/project-config-redis-compression branch July 21, 2022 14:14
jjbayer added a commit that referenced this pull request Jul 25, 2022
Record more metrics for the decompression of project config entries in
the redis cache (see #1345).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants