-
Notifications
You must be signed in to change notification settings - Fork 431
feat(server) added eviction strategy #1275
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(server) added eviction strategy #1275
Conversation
WalkthroughThe pull request introduces a configurable eviction strategy feature for the Interactsh server. A new CLI flag Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Parser
participant Opts as storage.Options
participant DB as StorageDB
participant Cache as TTL Cache
CLI->>Opts: Set EvictionStrategy (Sliding/Fixed)
rect rgba(200, 220, 255, 0.3)
Note over DB,Cache: Sliding Strategy (expire-after-access)
DB->>DB: EvictionStrategy == Sliding?
DB->>Cache: Configure ExpireAfterAccess
Cache->>Cache: Extend TTL on access
end
rect rgba(255, 200, 200, 0.3)
Note over DB,Cache: Fixed Strategy (expire-after-write)
DB->>DB: EvictionStrategy == Fixed?
DB->>Cache: Configure ExpireAfterWrite
Cache->>Cache: TTL fixed from write time
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 0
🧹 Nitpick comments (1)
pkg/storage/storagedb_test.go (1)
163-181: LGTM! Fixed eviction test correctly validates TTL independence from access.The test properly verifies that accessing an entry does NOT extend its TTL in fixed mode, confirming expire-after-write behavior.
Optional: Consider slightly larger timing buffers for test robustness.
The tests use 100ms TTL with 10ms buffers. While this should be sufficient, slightly larger buffers (e.g., 20-50ms) could reduce flakiness on slower CI systems. This is optional and not urgent since the current values are reasonable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)cmd/interactsh-server/main.go(2 hunks)pkg/options/server_options.go(1 hunks)pkg/storage/option.go(1 hunks)pkg/storage/storagedb.go(1 hunks)pkg/storage/storagedb_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
cmd/interactsh-server/main.go (1)
pkg/storage/option.go (4)
EvictionStrategy(5-5)EvictionStrategyFixed(9-9)EvictionStrategySliding(8-8)DefaultOptions(23-26)
pkg/options/server_options.go (1)
pkg/storage/option.go (1)
EvictionStrategy(5-5)
pkg/storage/storagedb_test.go (2)
pkg/storage/storagedb.go (1)
New(35-75)pkg/storage/option.go (4)
Options(12-17)EvictionStrategy(5-5)EvictionStrategySliding(8-8)EvictionStrategyFixed(9-9)
pkg/storage/storagedb.go (1)
pkg/storage/option.go (3)
EvictionStrategy(5-5)EvictionStrategyFixed(9-9)EvictionStrategySliding(8-8)
🔇 Additional comments (10)
README.md (1)
356-356: LGTM! Clear documentation of the new eviction strategy flag.The documentation follows the established format and clearly specifies the available options and default value.
pkg/options/server_options.go (1)
23-23: LGTM! Field addition is consistent with the existing pattern.The
EvictionStrategyfield captures CLI input and is mapped to storage options inmain.go, following the same pattern as theEvictionandNoEvictionfields.pkg/storage/storagedb.go (1)
41-48: LGTM! Eviction strategy implementation is correct.The switch correctly implements the two eviction strategies:
- Fixed: TTL counted from write time (expire-after-write)
- Sliding: TTL counted from last access (expire-after-access, backward compatible default)
pkg/storage/storagedb_test.go (1)
137-161: LGTM! Sliding eviction test correctly validates TTL extension on access.The test properly verifies that accessing an entry extends its TTL in sliding mode.
pkg/storage/option.go (3)
5-10: LGTM! Clean enum definition with clear documentation.The
EvictionStrategytype and constants are well-defined with helpful comments explaining the behavior of each strategy.
16-16: LGTM! Field addition enables configurable eviction strategy.
24-25: LGTM! Default to sliding strategy maintains backward compatibility.Setting
EvictionStrategySlidingas the default preserves the original expire-after-access behavior.cmd/interactsh-server/main.go (3)
51-51: LGTM! Flag definition follows established patterns.The eviction strategy flag is properly defined with a sensible default value.
223-232: LGTM! Robust parsing and validation of eviction strategy.The implementation correctly:
- Uses case-insensitive matching for user convenience
- Maps string values to typed constants
- Provides clear error messaging for invalid input
237-237: LGTM! Eviction strategy correctly propagated to storage options.
Solves #1272
Add
EvictionStrategyoption, allowingfixedeviction where entries are deleted after TTL passes (even if accessed)Tests
Summary by CodeRabbit
New Features
-es/-eviction-strategyto configure interaction data eviction behavior with two options: "sliding" (default, TTL extends on access) and "fixed" (TTL fixed from write time).Tests