Failed to generate code suggestions for PR

Greptile Overview

Greptile Summary

This PR rolls back usage of the scc (Scalable Concurrent Collections) crate across multiple modules, replacing it with standard parking_lot::RwLock or tokio::sync::RwLock wrapped HashMap/HashSet collections. This change addresses issue #9419 where searches were getting stuck after upgrading to version 0.20.1.

Key Changes:

• WAL file locking (src/common/infra/wal.rs): Replaced scc::HashMap<String, AtomicIsize> with parking_lot::RwLock<SearchingFileLocker>. Functions are now synchronous instead of async, with explicit reference counting for file locks.
• Local lock (src/infra/src/local_lock.rs): Changed from scc::HashMap with OccupiedEntry to RwLock<HashMap> with a new LockHolder struct. The API now requires a two-step lock acquisition pattern.
• File list storage (src/service/search/datafusion/storage/file_list.rs): Replaced scc::HashMap with parking_lot::RwLock<HashMap> for file and segment caching.
• Various job/service modules: Consistently replaced scc::HashSet with RwLock<HashSet> for tracking processing files and ongoing jobs.

Memory management improvements:

• Added shrink_to_fit() calls after removing elements to prevent memory accumulation
• Explicit lock dropping patterns to prevent potential deadlocks

The rollback from scc to standard locking primitives suggests that the lock-free concurrent collections were causing issues with the search request lifecycle, potentially related to how entries were being acquired and released in the flight search context.

Confidence Score: 4/5

• This PR addresses a critical P0 bug (search blocking) by rolling back to simpler, well-tested locking primitives.
• The changes are consistent across all files, replacing scc collections with standard RwLock patterns. The logic is straightforward and follows established Rust concurrency patterns. Tests are updated to match the new API. Minor concern about the local_lock API change requiring callers to use two-step locking, but the only caller (schema.rs) is updated correctly.
• The src/infra/src/local_lock.rs change removes the lock_multi function - verify no other callers depend on it.

Important Files Changed

File Analysis

Sequence Diagram

sequenceDiagram
    participant Client
    participant FlightHandler as Flight Handler
    participant WAL as WAL Module
    participant FileList as File List Storage
    participant SearchFiles as SEARCHING_FILES

    Client->>FlightHandler: Search Request
    FlightHandler->>WAL: lock_files(files)
    WAL->>SearchFiles: write().lock(file) for each file
    Note over SearchFiles: Reference count incremented

    FlightHandler->>WAL: lock_request(trace_id, files)
    WAL->>WAL: Store files in SEARCHING_REQUESTS

    FlightHandler->>FileList: set(trace_id, schema_key, files)
    FileList->>FileList: Store file metadata

    FlightHandler-->>Client: Stream Results

    Client->>FlightHandler: Request Complete/Error
    FlightHandler->>FlightHandler: clear_session_data(trace_id)
    FlightHandler->>FileList: clear(trace_id)
    FileList->>FileList: Remove entries + shrink_to_fit()

    FlightHandler->>WAL: release_request(trace_id)
    WAL->>WAL: Remove from SEARCHING_REQUESTS
    WAL->>SearchFiles: write().release(file) for each file
    Note over SearchFiles: Reference count decremented, removed if 0