shahrs87 commented on code in PR #1753:
URL: https://github.com/apache/phoenix/pull/1753#discussion_r1424498361
##########
phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java:
##########
@@ -650,16 +650,13 @@ public static Configuration
setUpConfigForMiniCluster(Configuration conf, ReadOn
}
/*
- Set property hbase.coprocessor.regionserver.classes to include
PhoenixRegionServerEndpoint
- by default. If some other regionserver coprocs are already present
then append
- PhoenixRegionServerEndpoint to the existing coprocs.
+ Set property hbase.coprocessor.regionserver.classes to include
PhoenixRegionServerEndpoint
+ by default, if some other regionserver coprocs are not already present.
*/
private static void setPhoenixRegionServerEndpoint(Configuration conf) {
String value = conf.get(REGIONSERVER_COPROCESSOR_CONF_KEY);
if (value == null) {
value = PhoenixRegionServerEndpoint.class.getName();
- } else {
Review Comment:
Earlier this was the check to decide whether we invalidate the cache on all
regionservers or not. In InvalidateMetadataCacheIT test, we override the
regionserver coproc to FailingPhoenixRegionServerEndpoint
[here](https://github.com/apache/phoenix/blob/PHOENIX-6883-feature/phoenix-core/src/it/java/org/apache/phoenix/end2end/InvalidateMetadataCacheIT.java#L51).
But we still add `PhoenixRegionServerEndpoint` to the conf property to pass
the below check. But now since the check is determined by a separate boolean
conf property, we can remove the unnecessary mess that I have created earlier.
```
String value = conf.get(REGIONSERVER_COPROCESSOR_CONF_KEY);
if (value == null
||
!value.contains(PhoenixRegionServerEndpoint.class.getName()))
```
--
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]
