pingtimeout commented on issue #761: URL: https://github.com/apache/polaris/issues/761#issuecomment-2782808679
@dennishuo looking again at this issue, I don't think the `Resolver` can solve any of the concurrency issues listed here. To me, it is clear that the downstream concurrency issues will propagate upstream. Here is what I could find: 1. The `Resolver` is not thread-safe either (no lock, no `synchronized` block, maintains mutable state) 2. `Resolver` instances are not shared between multiple callers, as there is one instance per REST API call 3. All `Resolver` instances share the same `EntityCache` instance. 4. The `bulkValidate` method assumes that by calling `getAndRefreshIfNeeded` is enough to refresh the entity ([L573, see code below](https://github.com/apache/polaris/blob/3603fa317ad8313f178c09137965bc0e5c7f256d/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java#L573-L580)), which has been proven to be false. ```java // refresh that entity. If versions is null, it has been dropped if (this.cache != null) { refreshedResolvedEntity = this.cache.getAndRefreshIfNeeded( this.polarisCallContext, entity, versions.getEntityVersion(), versions.getGrantRecordsVersion()); ``` Because of (2), any stress test that concurrently call different methods of `Resolver` would be irrelevant. The code does not allow that pattern to happen. In a way, it is a good thing because of the combination of (1) and (2). Now, (2) and (3) mean that the stress tests against the `EntityCache` **are** valid. And in combination with (4), we know that the Resolver propagates the issues to the user. I am going to cleanup the IndexedCache implementation a bit to make it work better with Java 11. Then I will open a PR that attempts to fix this issue. Although there will be some `TODO` elements for the `.primaryKey()` and `.secondaryKey()` method calls that use `null` for the Polaris context. -- 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]
