shuwenwei opened a new pull request, #18371:
URL: https://github.com/apache/iotdb/pull/18371

   ## Problem
   
   When multiple FragmentInstances share an `ExternalTsFileQueryResource`, a 
race condition can cause the resource to be closed prematurely, resulting in 
`"ExternalTsFileQueryResource has been closed"` exceptions.
   
   ### Trigger conditions:
   1. Query uses `read_tsfile(...)`, multiple Fragments share one 
`ExternalTsFileQueryResource`
   2. Many devices/timeseries, IoTDB splits into multiple Fragments by 
parallelism (e.g. `PARTITION BY timeseries_id`)
   3. Fragment startup is not synchronized:
      - Fragment A initializes first, reference count = 1
      - Fragment B is still in the scheduling queue, hasn't called `retain()`
   4. Fragment A finishes quickly and releases its reference, count becomes 0 → 
resource closed
   5. Fragment B then initializes and calls `retainFragmentInstanceUsage()` → 
throws `"ExternalTsFileQueryResource has been closed"`
   
   Another path: `QueryExecution` cleanup runs while no FragmentInstance has 
initialized yet (count=0), closing the resource before scheduled FIs start.
   
   ### Easier to trigger with:
   - Large number of timeseries/devices, producing many partitions
   - DataNode parallelism > 1
   - Uneven partition sizes (some Fragments complete quickly, others start late)
   - Many concurrent queries, Drivers queued in thread pool
   - High CPU/IO pressure, widening Fragment startup time gap
   - Query cancellation/error overlapping with Fragment scheduling
   
   ## Fix
   
   Introduce a **two-phase close mechanism**:
   
   - **`closeByFragmentInstance()`**: only decrements the usage count. It no 
longer closes the resource on its own — it waits for the QueryExecution signal.
   - **`closeByQueryExecution()`**: sets a `wantsClose` flag and closes only 
when the usage count reaches zero.
   - **`QueryExecution` state listener**: now calls 
`releaseExternalTsFileQueryResources()` for **all** terminal states (including 
`FINISHED`), not just error states, ensuring the resource is always eventually 
closed.
   
   ### Changes:
   | File | Change |
   |---|---|
   | `ExternalTsFileQueryResource.java` | Add `queryExecutionWantsToClose` 
flag; modify `closeByFragmentInstance` and `closeByQueryExecution` close 
conditions |
   | `QueryExecution.java` | Call `releaseExternalTsFileQueryResources()` for 
all terminal states |
   
   ### Why this approach:
   - **No blocking**: pure flag mechanism, no wait/notify
   - **Retry-safe**: retry creates new Resource objects, old ones close 
normally via existing FI releases
   - **No leaks**: FI failures that never call `retain()` don't affect the 
count, unlike a total-count approach
   - **Backward compatible**: single-Fragment behavior unchanged; `closeByQE` 
is idempotent on repeated calls


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to