albertogpz edited a comment on pull request #6874: URL: https://github.com/apache/geode/pull/6874#issuecomment-934499121
> Maybe the way to go here is to rewrite `QueryObserverHolder` as a class that hold a `ThreadLocal<QueryObserverHolder>` and then have ALL other product classes such as `DefaultQuery` just use `QueryObserverHolder` as an API for accessing that ThreadLocal. I like that idea. I did an experiment with it but saw many test cases fail: https://concourse.apachegeode-ci.info/builds/84932 The problem is that there are test cases that count on setting the observer globally and then running a query from the client. Take a look for example at: `SelectStarQueryDUnitTets::testSelectStarQueryForPartitionedRegion`. In these cases, you cannot set the thread local observer in the thread that will execute the query because the query is executed remotely from the client. I have been able to change the test cases in `QueryUsingFunctionContextDUnitTest` and set the observer in the function executing the queries instead of globally in the servers but this approach is not valid for the test cases previously mentioned. I am also worried about the use done by the `QueryObserver` to provide `trace` information in queries. It is not possible to set a query observer and at the same time have trace information. We could also go for having two variables in the `QueryObserverHolder`: a thread local one and the current global one. Setting the instance could set the value in the global one and getting the instance could check if the thread local has a value. If it does, it would return it. Otherwise, the thread local one would be set to the global one value and then be returned. That way, during a query execution, only the first call to `getInstance()` should require synchronization. What do you think? -- 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]
