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

chore: remove redis permanent storage #1917

Merged
merged 3 commits into from
Dec 13, 2024
Merged

chore: remove redis permanent storage #1917

merged 3 commits into from
Dec 13, 2024

Conversation

carneiro-cw
Copy link
Contributor

@carneiro-cw carneiro-cw commented Dec 13, 2024

PR Type

Enhancement


Description

  • Removed Redis as a permanent storage option from the project
  • Deleted the entire Redis implementation file (redis.rs)
  • Updated mod.rs to remove Redis-related imports, configurations, and options
  • Removed Redis from the list of storage implementations in README.md
  • Simplified the storage options to In Memory and RocksDB (default)

Changes walkthrough 📝

Relevant files
Configuration changes
mod.rs
Remove Redis storage option                                                           

src/eth/storage/permanent/mod.rs

  • Removed Redis-related imports and configurations
  • Removed Redis as a storage option from PermanentStorageKind enum
  • Updated init() function to remove Redis initialization
  • Updated from_str() implementation to remove Redis option
  • +0/-14   
    Enhancement
    redis.rs
    Remove Redis permanent storage implementation                       

    src/eth/storage/permanent/redis.rs

  • Entire file removed, containing Redis implementation for
    PermanentStorage
  • +0/-402 
    Documentation
    README.md
    Update README to remove Redis storage option                         

    README.md

    • Removed Redis from the list of current storage implementations
    +0/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Logging Improvement
    The initialization of permanent storage is logged, but it might be beneficial to add more detailed logging, especially for the RocksDB configuration.

    Error Handling
    The error handling for unknown storage types in the FromStr implementation could be more informative, possibly including the list of valid options.

    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @carneiro-cw carneiro-cw enabled auto-merge (squash) December 13, 2024 16:39
    @carneiro-cw carneiro-cw merged commit 37c4617 into main Dec 13, 2024
    34 checks passed
    @carneiro-cw carneiro-cw deleted the remove_redis branch December 13, 2024 16:47
    @gabriel-aranha-cw
    Copy link
    Contributor

    Final benchmark:
    Run ID: bench-932357993

    Git Info:

    Configuration:

    • Target Account Strategy: Default

    RPS Stats: Max: 1188.00, Min: 537.00, Avg: 970.07, StdDev: 71.25
    TPS Stats: Max: 1124.00, Min: 771.00, Avg: 941.13, StdDev: 71.41

    Plot: View Plot

    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.

    2 participants