Skip to content

Conversation

@nikhilsinhaparseable
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable commented Nov 6, 2025

Summary by CodeRabbit

Release Notes

No user-visible changes. This release contains internal code improvements with no impact on functionality or user experience.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

A static mutex lock in the query logstream handler is changed from private to public visibility, allowing external modules to access the synchronization primitive without modifying its behavior or logic.

Changes

Cohort / File(s) Summary
Visibility modifier
src/handlers/http/modal/query/querier_logstream.rs
CREATE_STREAM_LOCK static changed from private to pub

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Verify that exposing this lock does not introduce unintended synchronization side effects or external dependencies
  • Confirm the lock's purpose remains clear and its usage pattern is appropriate for public access

Poem

🔓 A lock once hidden in the shade,
Now opens to the world, unafraid,
No logic bent, no flow rearranged,
Just visibility—tastefully changed! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request has no description while the repository template requires description, testing checklist, and documentation sections. Add a description following the repository template including: the goal, rationale, key changes, and confirmation of testing and documentation.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: making a static lock public for enterprise module usage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 621f7ef and 6dea16e.

📒 Files selected for processing (1)
  • src/handlers/http/modal/query/querier_logstream.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-02-14T09:49:25.818Z
Learnt from: de-sh
Repo: parseablehq/parseable PR: 1185
File: src/handlers/http/logstream.rs:255-261
Timestamp: 2025-02-14T09:49:25.818Z
Learning: In Parseable's logstream handlers, stream existence checks must be performed for both query and standalone modes. The pattern `!PARSEABLE.streams.contains(&stream_name) && (PARSEABLE.options.mode != Mode::Query || !PARSEABLE.create_stream_and_schema_from_storage(&stream_name).await?)` ensures proper error handling in both modes.

Applied to files:

  • src/handlers/http/modal/query/querier_logstream.rs
📚 Learning: 2025-10-21T02:22:24.403Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1448
File: src/parseable/mod.rs:419-432
Timestamp: 2025-10-21T02:22:24.403Z
Learning: In Parseable's internal stream creation (`create_internal_stream_if_not_exists` in `src/parseable/mod.rs`), errors should not propagate to fail server initialization. The function creates both pmeta and pbilling internal streams, and failures are logged but the function always returns `Ok(())` to ensure server startup resilience. Individual stream creation failures should not prevent syncing of successfully created streams.

Applied to files:

  • src/handlers/http/modal/query/querier_logstream.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Build Default x86_64-unknown-linux-gnu
  • GitHub Check: Build Default aarch64-unknown-linux-gnu
  • 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 Kafka aarch64-apple-darwin
  • GitHub Check: Build Kafka x86_64-unknown-linux-gnu
  • GitHub Check: Quest Smoke and Load Tests for Distributed deployments
  • GitHub Check: Quest Smoke and Load Tests for Standalone deployments
  • GitHub Check: coverage
🔇 Additional comments (1)
src/handlers/http/modal/query/querier_logstream.rs (1)

32-32: Approve the visibility change; verify external usage contract with enterprise module.

The visibility change is functionally safe within the current codebase. The lock is acquired only at line 120 of put_stream() and released immediately—no secondary locks are acquired while holding it.

However, since this exposes the lock to external modules (the enterprise module), you must verify:

  1. Lock ordering: Ensure the enterprise module never acquires this lock alongside other locks in different orderings elsewhere in its code.
  2. Documentation: Add rustdoc to CREATE_STREAM_LOCK documenting what it protects and usage guidelines.

The codebase has other locks (UPDATE_LOCK, ALERTS, REFERENCE_TIMESTAMP), but none are combined with CREATE_STREAM_LOCK internally, so immediate deadlock risk is low.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nikhilsinhaparseable nikhilsinhaparseable merged commit a91a3cc into parseablehq:main Nov 6, 2025
13 checks passed
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.

1 participant