-
Notifications
You must be signed in to change notification settings - Fork 3
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
Redis updates #161
Redis updates #161
Conversation
WalkthroughThis update focuses on optimizing resource allocations across various configurations and enhancing documentation clarity. Changes include increased CPU and memory for Redis and Memcached, autoscaling adjustments in YAML templates, and improved formatting in Redis migration documentation. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- docs/redis.md (6 hunks)
- templates/memcached.yml (2 hunks)
- templates/postgres.yml (1 hunks)
- templates/redis.yml (1 hunks)
- templates/redis2.yml (1 hunks)
Files skipped from review due to trivial changes (2)
- templates/postgres.yml
- templates/redis.yml
Additional Context Used
LanguageTool (4)
docs/redis.md (4)
Near line 11: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ication to pick up all changes (usually very quick), useINFO
orDBSIZE
to check progr...
Near line 22: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...handling by setting TLS verification tonone
, otherwise most apps are not able to connect. 2. ...
Near line 22: As a shorter alternative for ‘able to’, consider using “can not”.
Context: ...fication tonone
, otherwise most apps are not able to connect. 2. We are moving to private R...
Near line 26: Unpaired symbol: ‘]’ seems to be missing
Context: ...e tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/...
Additional comments not posted (6)
templates/memcached.yml (2)
7-8
: Increased CPU and memory allocations are appropriate for handling higher loads.
18-18
: Autoscaling disabled. Verify the impact on system performance and stability.Verification successful
Autoscaling disabled across multiple services as part of recent updates. The change in
memcached.yml
aligns with these updates. It's important to monitor the impact on system performance and stability as these changes roll out.
- Relevant Files:
templates/memcached.yml
templates/postgres.yml
templates/redis2.yml
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any related issues or performance metrics indicating problems due to disabled autoscaling. # Test: Search for issues related to autoscaling in the repository. rg --type yaml 'autoscaling: disabled'Length of output: 40
Script:
#!/bin/bash # Description: Broaden the search to capture more general mentions of autoscaling settings in YAML files. # Search for general mentions of autoscaling in YAML files to understand if similar settings exist or discussions around such changes. rg --type yaml 'autoscaling' --files-with-matchesLength of output: 321
Script:
#!/bin/bash # Description: Check for recent commits mentioning autoscaling to see if there's any discussion or rationale provided for disabling it. # Search for recent commits that mention 'autoscaling' to gather context on the decision-making process. git log -G 'autoscaling' --since="1 month ago" --pretty=format:"%h - %s"Length of output: 133
Script:
#!/bin/bash # Description: Examine the autoscaling settings in other YAML files to compare with the disabled setting in memcached.yml. # Print the lines containing autoscaling settings from each YAML file to understand their configurations. rg --type yaml 'autoscaling' templates/*.yml --context 2Length of output: 2014
Script:
#!/bin/bash # Description: Check the details of the Redis commit to see if it provides context about autoscaling changes. # Show the detailed commit message and changed files for the Redis commit related to autoscaling. git show 9cd7951Length of output: 4916
templates/redis2.yml (3)
1-6
: Volume set configuration is well-defined and suitable for general-purpose Redis instances.
10-28
: Container settings and resource allocations are well-tailored for a basic persistent Redis setup.
30-33
: Autoscaling disabled. Verify the impact on system performance and stability.Verification successful
Autoscaling is disabled across multiple service configurations (memcached, PostgreSQL, Redis). No issues or performance metrics indicating problems due to this configuration were found in the repository.
templates/memcached.yml
templates/postgres.yml
templates/redis2.yml
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any related issues or performance metrics indicating problems due to disabled autoscaling. # Test: Search for issues related to autoscaling in the repository. rg --type yaml 'autoscaling: disabled'Length of output: 40
Script:
#!/bin/bash # Description: Broaden the search for discussions or logs related to autoscaling issues in non-YAML files. # Search in all file types for mentions of "autoscaling" issues. rg 'autoscaling' --files-with-matches | xargs rg 'disabled'Length of output: 194
docs/redis.md (1)
1-1
: Conversion of bolded text to header tags enhances document structure and readability.Also applies to: 19-19, 28-28, 42-42, 58-58, 68-68, 95-95
9cd7951
to
8e75b87
Compare
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.
Actionable comments posted: 2
Out of diff range and nitpick comments (2)
docs/redis.md (2)
Line range hint
38-38
: Ensure code blocks are surrounded by blank lines for proper formatting.+ ```sh heroku redis:keyspace-notifications -c KA -a my-app
--- Line range hint `105-105`: Ensure code blocks are surrounded by blank lines for proper formatting. ```diff + Setting commit interval to default value (1) Setting commit interval to default value (1) Job: [SimpleJob: [name=snapshot-replication]] launched with the following parameters: [{}] Executing step: [snapshot-replication] Scanning 0% ╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/8891 (0:00:00 / ?)Job: [SimpleJob: [name=scan-reader]] launched with the following parameters: [{}] Executing step: [scan-reader] Scanning 61% ━━━━━━━━━━━━━━━━╸━━━━━━━━━━ 5460/8891 (0:00:07 / 0:00:04) 780.0/sStep: [scan-reader] executed in 7s918ms Closing with items still in queue Job: [SimpleJob: [name=scan-reader]] completed with the following parameters: [{}] and the following status: [COMPLETED] in 7s925ms Scanning 100% ━━━━━━━━━━━━━━━━━━━━━━━━━━━ 9482/9482 (0:00:11 / 0:00:00) 862.0/s Step: [snapshot-replication] executed in 13s333ms Executing step: [verification] Verifying 0% ╺━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/8942 (0:00:00 / ?)Job: [SimpleJob: [name=RedisItemReader]] launched with the following parameters: [{}] Executing step: [RedisItemReader] Verifying 2% ╺━━━━━━━━━━━━━━━━━ 220/8942 (0:00:00 / 0:00:19) ?/s >0 T0 ≠Step: [RedisItemReader] executed in 7s521ms Closing with items still in queue Job: [SimpleJob: [name=RedisItemReader]] completed with the following parameters: [{}] and the following status: [COMPLETED] in 7s522ms Verification completed - all OK Step: [verification] executed in 7s776ms Job: [SimpleJob: [name=snapshot-replication]] completed with the following parameters: [{}] and the following status: [COMPLETED] in 21s320ms
</blockquote></details></blockquote></details> <details> <summary>Review Details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 9cd7951ad9ea3528c73f5facfa7fa74f38910622 and 8e75b875e6e7addb24c98a1335c69d128d837a25. </details> <details> <summary>Files selected for processing (5)</summary> * docs/redis.md (6 hunks) * templates/memcached.yml (2 hunks) * templates/postgres.yml (1 hunks) * templates/redis.yml (1 hunks) * templates/redis2.yml (1 hunks) </details> <details> <summary>Files skipped from review as they are similar to previous changes (4)</summary> * templates/memcached.yml * templates/postgres.yml * templates/redis.yml * templates/redis2.yml </details> <details> <summary>Additional Context Used</summary> <details><summary>LanguageTool (4)</summary><blockquote> <details><summary>docs/redis.md (4)</summary><blockquote> Near line 11: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase. Context: ...ication to pick up all changes (usually very quick), use `INFO` or `DBSIZE` to check progr... --- Near line 22: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence. Context: ...handling by setting TLS verification to `none`, otherwise most apps are not able to connect. 2. ... --- Near line 22: As a shorter alternative for ‘able to’, consider using “can not”. Context: ...fication to `none`, otherwise most apps are not able to connect. 2. We are moving to private R... --- Near line 26: Unpaired symbol: ‘]’ seems to be missing Context: ...e tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/... </blockquote></details> </blockquote></details> </details> <details><summary>Additional comments not posted (5)</summary><blockquote> <details><summary>docs/redis.md (5)</summary><blockquote> `1-1`: Updated section title broadens the scope and inclusivity. --- `19-19`: Heading format changed to subheading for better document structure. --- `28-28`: Heading format changed to subheading for better document structure. --- `42-42`: Heading format changed to subheading for better document structure. --- `95-95`: Heading format changed to subheading for better document structure. </blockquote></details></blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
``` | ||
|
||
**Useful Redis CLI commands:** | ||
### Useful Redis CLI commands: | ||
|
||
Quick-check keys qty: | ||
``` |
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.
Ensure code blocks are surrounded by blank lines for proper formatting.
+
```sh
redis-cli -u MY_CONTROL_PLANE_REDIS_URL -p 6379
<!-- This is an auto-generated comment by CodeRabbit -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 7
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/redis.md (5 hunks)
Additional Context Used
LanguageTool (1)
docs/redis.md (1)
Near line 26: Unpaired symbol: ‘]’ seems to be missing
Context: ...e tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/...
Additional comments not posted (5)
docs/redis.md (5)
1-1
: Update to section heading aligns better with content scope.
11-11
: Corrected adverb usage enhances the formal tone of the document.
19-19
: Section heading updated to a more structured format.
22-22
: Grammar correction improves clarity and formality.
58-58
: Section heading updated to a more structured format.
There are two templates examples in this repo: | ||
- `redis` - basic non-persistent template. It is good for review-apps or staging or where no persistence is required | ||
- `redis2` - basic persistent template. Good for production where persistence is needed, but cluster is overkill. |
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.
List items should be surrounded by blank lines for better readability.
+
There are two templates examples in this repo:
- `redis` - basic non-persistent template. It is good for review-apps or staging or where no persistence is required
- `redis2` - basic persistent template. Good for production where persistence is needed, but cluster is overkill.
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
There are two templates examples in this repo: | |
- `redis` - basic non-persistent template. It is good for review-apps or staging or where no persistence is required | |
- `redis2` - basic persistent template. Good for production where persistence is needed, but cluster is overkill. | |
There are two templates examples in this repo: | |
- `redis` - basic non-persistent template. It is good for review-apps or staging or where no persistence is required | |
- `redis2` - basic persistent template. Good for production where persistence is needed, but cluster is overkill. | |
|
||
2. We are moving to private Redis that don't have a public URL, so have to do it from a Control Plane GVC container. | ||
|
||
The tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/riot/riot-redis/index.html) | ||
|
||
**Heroku Redis:** | ||
### Heroku Redis: |
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.
Ensure code blocks are surrounded by blank lines for proper formatting.
+
### Heroku Redis:
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
### Heroku Redis: | |
### Heroku Redis: | |
@@ -23,7 +39,7 @@ | |||
heroku redis:cli -a my-app | |||
``` | |||
|
|||
**Control Plane Redis:** | |||
### Control Plane Redis: |
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.
Ensure code blocks are surrounded by blank lines for proper formatting.
+
### Control Plane Redis:
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
### Control Plane Redis: | |
### Control Plane Redis: | |
@@ -36,10 +52,10 @@ | |||
apt-get install redis -y | |||
|
|||
# connect to local cloud Redis | |||
redis-cli -u MY_CONTROL_PLANE_REDIS_URL | |||
redis-cli -u MY_CONTROL_PLANE_REDIS_URL -p 6379 |
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.
Specify the language for the code block to enhance syntax highlighting.
+ ```bash
55~
redis-cli -u MY_CONTROL_PLANE_REDIS_URL -p 6379
56~
+ ```
@@ -49,7 +65,7 @@ | |||
db0:keys=9496,expires=2941,avg_ttl=77670114535 | |||
``` | |||
|
|||
**Create a Control Plane sync workload** | |||
### Create a Control Plane sync workload |
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.
Ensure code blocks are surrounded by blank lines for proper formatting.
+
### Create a Control Plane sync workload
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
### Create a Control Plane sync workload | |
### Create a Control Plane sync workload |
@@ -76,7 +92,7 @@ | |||
live | |||
``` | |||
|
|||
**Sync process** | |||
### Sync process |
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.
Ensure code blocks are surrounded by blank lines for proper formatting.
+
### Sync process
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
### Sync process |
Sync process
</details>
<!-- suggestion_end -->
<!-- This is an auto-generated comment by CodeRabbit -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
Out of diff range and nitpick comments (3)
docs/redis.md (3)
Line range hint
28-42
: Specific commands for managing Heroku Redis.+ ```sh heroku redis:keyspace-notifications -c KA -a my-app
--- Line range hint `42-55`: Commands for managing Control Plane Redis. ```diff + ```sh # open cpl interactive shell cpl run bash -a my-app # install redis CLI if you don't have it in Docker apt-get update apt-get install redis -y # connect to local cloud Redis redis-cli -u MY_CONTROL_PLANE_REDIS_URL -p 6379
--- Line range hint `68-95`: Detailed setup and execution guide for a Control Plane sync workload. ```diff + ```sh name: riot-redis suspend: true min/max scale: 1/1 firewall: all firewalls off image: fieldengineering/riot-redis CPU: 1 Core RAM: 1 GB command args: --info -u rediss://...your_heroku_redis_url... --tls-verify=NONE replicate -h ...your_control_plane_redis_host... --mode live
</blockquote></details></blockquote></details> <details> <summary>Review Details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 68ea5906122df182c6632c96137141c3ac6b202b and 58dd5605617ecf0fab35012177e8119015a225d3. </details> <details> <summary>Files selected for processing (1)</summary> * docs/redis.md (5 hunks) </details> <details> <summary>Additional Context Used</summary> <details><summary>LanguageTool (2)</summary><blockquote> <details><summary>docs/redis.md (2)</summary><blockquote> Near line 22: Unpaired symbol: ‘]’ seems to be missing Context: ...e tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/... --- Near line 26: Unpaired symbol: ‘]’ seems to be missing Context: ...e tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/... </blockquote></details> </blockquote></details> </details> <details><summary>Additional comments not posted (3)</summary><blockquote> <details><summary>docs/redis.md (3)</summary><blockquote> `1-1`: Section title broadened to encompass general Redis migrations. --- `3-5`: Clarification on Redis templates enhances understanding of their use cases. --- `7-15`: Detailed guide on using SLAVEOF for Redis migration. </blockquote></details></blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
## Option 2: use Redis-RIOT (harder way, where option 1 is not possible) | ||
|
||
### General considerations: | ||
|
||
1. Heroku uses self-signed TLS certificates, which are not verifiable. It needs special handling by setting | ||
TLS verification to `none`, otherwise most apps are not able to connect. | ||
The tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/riot/riot-redis/index.html) |
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.
Introduction of Redis-RIOT for complex Redis migrations.
- The tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/riot/riot-redis/index.html)
+ The tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/riot/riot-redis/index.html)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
## Option 2: use Redis-RIOT (harder way, where option 1 is not possible) | |
### General considerations: | |
1. Heroku uses self-signed TLS certificates, which are not verifiable. It needs special handling by setting | |
TLS verification to `none`, otherwise most apps are not able to connect. | |
The tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/riot/riot-redis/index.html) | |
## Option 2: use Redis-RIOT (harder way, where option 1 is not possible) | |
### General considerations: | |
1. Heroku uses self-signed TLS certificates, which are not verifiable. It needs special handling by setting | |
The tool that satisfies those criteria is [Redis-RIOT](https://developer.redis.com/riot/riot-redis/index.html) |
Summary by CodeRabbit
redis2.yml
for setting up a Redis instance with specific container settings and firewall configurations.memcached.yml
,postgres.yml
, andredis.yml
for better performance.