SebastianGruza commented on PR #2994: URL: https://github.com/apache/hugegraph/pull/2994#issuecomment-5702414353
Results for this head on a master that carries #3184, as asked in https://github.com/apache/hugegraph/pull/3184#issuecomment-5701198188. The branch is not rebased yet, so the tree under test is a local merge: `1a15e762` + `459a2b2f` (merge `a4c024d4`, no conflicts); baseline is plain `1a15e762`. Same lab and method as the earlier reports (PD + 3 stores, hstore server vs rocksdb oracle, results compared as id sets, full wipe between sides). Report with every file: https://github.com/SebastianGruza/hugegraph-validation/blob/master/reports/pr-2994/459a2b2f/README.md **Backend axis (hstore vs rocksdb, same build)** | build | suite (174 cases) | hg_j8 (207 cases) | |---|---|---| | `1a15e762` | OK=168, MISMATCH=0, TARGET-ERR=0, BOTH-ERR=6 | OK=182, MISMATCH=0, TARGET-ERR=0, BOTH-ERR=25 | | `1a15e762` + this head | OK=170, MISMATCH=0, TARGET-ERR=0, BOTH-ERR=4 | OK=190, **MISMATCH=6, TARGET-ERR=6**, BOTH-ERR=5 | The 37 store-side decode errors that section S used to produce on master are gone on plain `1a15e762` (#3184 confirmed on a fresh cluster). The PR turns two `without()` suite shapes and 21 J8 shapes from errors into results, and the 60 `hasLabel(neq('person'))` J8 shapes from 0 rows to the right rows, identically on both backends; `hasKey('nope').hasLabel(neq(...))` becomes the documented capacity fallback error. All as in the `2d53a55` report. **New on this head, hstore only: paged range-index queries from Gremlin fail with a sandbox error.** Six `G PAGE` positive controls (`g.V().has('score',gte(40))` without a label and `hasLabel('robot').has('age',gte(60))`, page sizes 7 / 50 / 500 through `~page`) fail on hstore with the PR and pass on rocksdb with the PR, on master hstore warm, and on master hstore fresh with J8 as the first traffic (0 sandbox warnings in that log). A second run on the warm PR server reproduces the same six. The server answers `500 Not allowed to access thread group via Gremlin`; stack: ``` HugeSecurityManager.checkAccess(HugeSecurityManager.java:169) java.lang.Thread.<init> org.apache.hugegraph.store.client.util.ExecutorPool$DefaultThreadFactory.newThread(ExecutorPool.java:64) ThreadPoolExecutor.execute / ExecutorCompletionService.submit org.apache.hugegraph.store.client.OrderedKvIterator.initialize(OrderedKvIterator.java:200) org.apache.hugegraph.store.client.OrderedKvIterator.hasNext(OrderedKvIterator.java:93) HstoreSessionsImpl$ColumnIterator.<init>(HstoreSessionsImpl.java:245) HstoreSessionsImpl$HstoreSession.scanOrdered(HstoreSessionsImpl.java:745) HstoreTable.queryByRange(HstoreTable.java:643) <- queryBy <- query <- HstoreStore.query ``` With the PR these queries reach `HstoreSession.scanOrdered()`, whose `OrderedKvIterator` creates the store client's `ExecutorPool` worker threads lazily on first use, and `HugeSecurityManager.checkAccess(ThreadGroup)` forbids thread creation from a Gremlin thread; the whitelist there (`callFromCaffeine`, `callFromAsyncTasks`, `callFromEventHubNotify`, `callFromBackendHbase`, `callFromRaft`, `callFromSofaRpc`) has no entry for `org.apache.hugegraph.store.client`. On master the same shapes do not take the ordered scan from a Gremlin thread, so the pool is never created under the sandbox. Two fixes come to mind, both outside this PR's diff: start the `ExecutorPool` eagerly when the store client is created, or whitelist the store client package the way raft and sofa-rpc are. @contrueCT I can retest the moment the branch is rebased, and if you prefer the whitelist route I can send that as a small PR on master so it does not block this one. -- 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]
