contrueCT opened a new pull request, #3213:
URL: https://github.com/apache/hugegraph/pull/3213

   ## Purpose of the PR
   
   Fix the ordered-scan worker-creation failure reported in 
[#2994](https://github.com/apache/hugegraph/pull/2994#issuecomment-5702414353), 
independently of its LABEL-query changes. This branch starts from master 
`1a15e762`.
   
   HStore's `OrderedKvIterator` primes multiple scan sources with a bounded 
executor. Its workers are created lazily and expire when idle. Creating a 
worker under Gremlin evaluation can raise `SecurityException: Not allowed to 
access thread group via Gremlin`. Merely constructing or warming the pool at 
startup does not cover later worker expiry.
   
   ## Main Changes
   
   - If executor submission throws `SecurityException`, initialize the 
remaining sources on the calling thread for that iterator. Stop retrying the 
denied executor, and drain any already-submitted tasks before completing 
initialization.
   - Keep concurrent initialization when submission succeeds, as well as sorted 
merging, limits, positions, and source cleanup. Preserve the caller's interrupt 
flag when inline initialization fails through interruption. If cancelling an 
existing task is also denied, record that exception and still close the 
sources. Security exceptions from the data source still propagate; they are not 
mistaken for submission failures.
   - Do not change `HugeSecurityManager`, add package allowlists, or grant 
scripts thread-creation privileges. The fallback may increase first-result 
latency when a sandbox denies worker creation; no performance improvement is 
claimed.
   
   The diagram shows the changed failure path; no new privileges are introduced:
   
   ```mermaid
   flowchart TB
       G[Gremlin evaluation] --> O[OrderedKvIterator initialization]
       O --> S[Submit source initialization]
       S --> Q{Submission permitted?}
       Q -->|Yes| W[Bounded worker initialization]
       Q -->|SecurityException - before| E[Query fails]
       Q -->|SecurityException - after| C[Initialize remaining sources on 
caller]
       W --> D[Drain submitted tasks]
       C --> D
       D --> M[Sorted merge with existing limit and cursor]
       U[Script creates a thread directly] --> B[Still denied by sandbox]
   ```
   
   ## Verifying these changes
   
   - [x] Need tests and can be verified as follows:
   
   The failure was reproduced on #2994 after merging master (`bcb8c1f3`), using 
the real `GremlinGroovyScriptEngine.eval()`, `HugeSecurityManager`, and 
production `OrderedKvIterator`, with controlled scan-source iterators. The new 
security tests also cover a worker expiring before the next query and confirm 
direct thread creation, including through the store-client thread factory, 
remains denied. They are registered in `UnitTestSuite`.
   
   Iterator regressions cover denied submission before/after a prior 
submission, a still-running source, sorted results, source errors, source 
security exceptions, interruption, and closure even when cancellation is 
denied. Existing concurrency, saturation, limit, and cursor tests remain in the 
suite.
   
   Passed on the SSH test host with Java 11: 15 security tests and 20 unique 
iterator tests, with no failures or skips. Formatting and the all-module clean 
compilation also passed. Commands (without style or coverage skips):
   
   ```bash
   mvn clean test -pl hugegraph-server/hugegraph-test -am -P unit-test,memory \
     -Dsurefire.failIfNoSpecifiedTests=false \
     -Dtest=OrderedScanSecurityTest,SecurityManagerTest
   mvn clean test -pl hugegraph-store/hg-store-test -am \
     -Dsurefire.failIfNoSpecifiedTests=false -Dtest=OrderedKvIteratorTest
   mvn editorconfig:format
   mvn clean compile -Dmaven.javadoc.skip=true
   ```
   
   This is an isolated sandbox/iterator reproduction and regression suite, not 
a rerun of the reporter's complete PD + three-store HTTP paging matrix. That 
matrix should be repeated after this fix is integrated into #2994. CI results 
are not claimed here.
   
   ## Does this PR potentially affect the following parts?
   
   - [ ] Dependencies
   - [ ] Modify configurations
   - [ ] The public API
   - [x] Other affects: ordered-scan initialization falls back to the caller 
when worker submission is denied.
   - [ ] Nope
   
   ## Documentation Status
   
   - [ ] `Doc - TODO`
   - [ ] `Doc - Done`
   - [x] `Doc - No Need`
   
   No public API or configuration change. The internal fallback and its latency 
trade-off are described above and in code.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to