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

Store Real Time Streamer Messages in Redis #241

Merged
merged 11 commits into from
Oct 5, 2023
Merged

Store Real Time Streamer Messages in Redis #241

merged 11 commits into from
Oct 5, 2023

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Sep 27, 2023

The streamer message is used by both the coordinator and runner. However, both currently poll the message from S3. There is a huge latency impact for pulling the message from S3. In order to improve this, the streamer message will now be cached in Redis with a TTL and pulled by runner from Redis. Only if there is a cache miss will runner pull from S3 again.

Pulling from S3 currently takes up 200-500ms, which is roughly 80-85% of the overall execution time of a function in runner. By caching the message, a cache hit leads to loading the data in 1-3ms in local testing, which corresponds to about 3-5% of the execution time, or a 1100% improvement in latency. The reduction of network related activity to such a low percentage of execution time also reduces the variability of a function's execution time greatly. Cache hits and misses will be logged for further tuning of TTL to reduce cache misses. In addition, processing the block takes around 1-3ms. This processing can be moved to be done before caching, saving 1-3ms each time that block is read from cache, which does stack up over time. The improvement there will be more important for historical backfill, which is planned to be cached soon.

Tracking Issue: #262
Parent Issue: #204

@darunrs
Copy link
Collaborator Author

darunrs commented Sep 27, 2023

#204

@darunrs darunrs linked an issue Sep 27, 2023 that may be closed by this pull request
@darunrs darunrs linked an issue Sep 29, 2023 that may be closed by this pull request
@darunrs darunrs changed the title Store Streamer Messages in Redis Store Real Time Streamer Messages in Redis Sep 29, 2023
@darunrs darunrs marked this pull request as ready for review October 2, 2023 19:19
@darunrs darunrs requested a review from a team as a code owner October 2, 2023 19:19
@darunrs darunrs requested a review from roshaans October 2, 2023 19:19
Copy link
Collaborator

@gabehamilton gabehamilton left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@morgsmccauley morgsmccauley left a comment

Choose a reason for hiding this comment

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

Great first start :) I've left a few comments. Can you please also link the issue?

indexer/storage/src/lib.rs Outdated Show resolved Hide resolved
indexer/storage/src/lib.rs Outdated Show resolved Hide resolved
indexer/queryapi_coordinator/src/main.rs Outdated Show resolved Hide resolved
indexer/queryapi_coordinator/src/utils.rs Outdated Show resolved Hide resolved
indexer/queryapi_coordinator/src/utils.rs Outdated Show resolved Hide resolved
runner/src/metrics.ts Outdated Show resolved Hide resolved
runner/src/metrics.ts Outdated Show resolved Hide resolved
runner/src/indexer/indexer.ts Outdated Show resolved Hide resolved
runner/package.json Show resolved Hide resolved
runner/src/indexer/indexer.ts Outdated Show resolved Hide resolved
@darunrs
Copy link
Collaborator Author

darunrs commented Oct 2, 2023

Tracking Issue: #262
Parent Issue: #204

Copy link
Collaborator

@morgsmccauley morgsmccauley left a comment

Choose a reason for hiding this comment

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

Couple small comments but otherwise looks good!

indexer/storage/src/lib.rs Outdated Show resolved Hide resolved
indexer/storage/src/lib.rs Outdated Show resolved Hide resolved
indexer/queryapi_coordinator/src/main.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@morgsmccauley morgsmccauley left a comment

Choose a reason for hiding this comment

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

LGTM!

@darunrs darunrs merged commit 9678087 into main Oct 5, 2023
6 checks passed
@darunrs darunrs deleted the cacheStreamer branch October 5, 2023 23:50
darunrs added a commit that referenced this pull request Oct 30, 2023
The streamer message is used by both the coordinator and runner.
However, both currently poll the message from S3. There is a huge
latency impact for pulling the message from S3. In order to improve
this, the streamer message will now be cached in Redis with a TTL and
pulled by runner from Redis. Only if there is a cache miss will runner
pull from S3 again.

Pulling from S3 currently takes up 200-500ms, which is roughly 80-85% of
the overall execution time of a function in runner. By caching the
message, a cache hit leads to loading the data in 1-3ms in local
testing, which corresponds to about 3-5% of the execution time, or a
1100% improvement in latency.

The reduction of network related activity
to a much lower percentage of execution time also reduces the variability
of a function's execution time greatly. Cache hits and misses will be
logged for further tuning of TTL to reduce cache misses. In addition,
processing the block takes around 1-3ms. This processing has been moved to
be done before caching, saving an extra 1-3ms each time that block is read from
cache. The improvement there will be important for historical backfill, which is planned to be optimized soon.

Tracking Issue: #262
Parent Issue: #204
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.

Cache Real Time Streamer Message in Redis
3 participants