fix: delete date stats #1306
fix: delete date stats #1306
Conversation
WalkthroughThe changes update error handling and logic for stream deletion and metric removal across several modules. In the HTTP logstream handlers, error messages now include specific error details when directory removal fails, using a more explicit error-capturing pattern. In the ingestor logstream handler, the pre-deletion existence check and recreation logic are removed, with deletion now proceeding directly if the stream is present in memory. In the stats module, the logic for deleting metrics by label prefix is revised to check for the presence of prefix elements anywhere in the label values, rather than requiring a positional prefix match. Additionally, a helper function was introduced to handle label value removals with error logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP_Handler
participant StreamMemory
participant FileSystem
participant Stats
Client->>HTTP_Handler: DELETE /stream/{name}
HTTP_Handler->>StreamMemory: Retrieve stream directory
alt Stream exists
HTTP_Handler->>FileSystem: Remove directory at data_path
alt Remove fails
HTTP_Handler->>HTTP_Handler: Log warning with error details
end
HTTP_Handler->>StreamMemory: Remove stream from memory
HTTP_Handler->>Stats: Delete metrics with label prefix
HTTP_Handler-->>Client: Respond with result
else Stream missing
HTTP_Handler-->>Client: Respond with error
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/handlers/http/modal/ingest/ingestor_logstream.rs (1)
64-71:⚠️ Potential issueSimplified logic with improved error handling
The updated code makes two significant improvements:
- Uses
get_streamwhich returns an error if the stream doesn't exist instead of trying to create it- Captures and logs the specific error when removing the stream directory
This change ensures that statistics deletion is properly tied to stream existence and improves error diagnostics.
This change is essential for fixing the main issue where statistics were not being deleted during stream deletion, as mentioned in the PR description. The previous implementation might have failed silently if the stream wasn't found in memory.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/handlers/http/logstream.rs(1 hunks)src/handlers/http/modal/ingest/ingestor_logstream.rs(1 hunks)src/handlers/http/modal/query/querier_logstream.rs(1 hunks)src/stats.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
🔇 Additional comments (2)
src/handlers/http/logstream.rs (1)
60-63: Improved error handling with detailed error messagesThe updated code now captures and logs the specific error when removing the stream directory, which provides better diagnostics for troubleshooting file system issues.
src/handlers/http/modal/query/querier_logstream.rs (1)
71-74: Improved error handling with detailed error messagesThe updated code now captures and logs the specific error when removing the stream directory, which provides better diagnostics for troubleshooting file system issues.
Stats were not getting deleted during stream deletion
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit