Jackie-Jiang commented on code in PR #12931:
URL: https://github.com/apache/pinot/pull/12931#discussion_r1569757400
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/BrokerRoutingManager.java:
##########
@@ -245,21 +245,27 @@ private void processInstanceConfigChange() {
Set<String> enabledServers = new HashSet<>();
List<String> newEnabledServers = new ArrayList<>();
for (ZNRecord instanceConfigZNRecord : instanceConfigZNRecords) {
+ // Put instance initialization logics into try-catch block to prevent
bad server configs affecting the entire
+ // cluster
String instanceId = instanceConfigZNRecord.getId();
- if (isEnabledServer(instanceConfigZNRecord)) {
- enabledServers.add(instanceId);
-
- // Always refresh the server instance with the latest instance config
in case it changes
- InstanceConfig instanceConfig = new
InstanceConfig(instanceConfigZNRecord);
- ServerInstance serverInstance = new ServerInstance(instanceConfig);
- if (_enabledServerInstanceMap.put(instanceId, serverInstance) == null)
{
- newEnabledServers.add(instanceId);
-
- // NOTE: Remove new enabled server from excluded servers because the
server is likely being restarted
- if (_excludedServers.remove(instanceId)) {
- LOGGER.info("Got excluded server: {} re-enabled, including it into
the routing", instanceId);
+ try {
+ if (isEnabledServer(instanceConfigZNRecord)) {
+ enabledServers.add(instanceId);
+
+ // Always refresh the server instance with the latest instance
config in case it changes
+ InstanceConfig instanceConfig = new
InstanceConfig(instanceConfigZNRecord);
+ ServerInstance serverInstance = new ServerInstance(instanceConfig);
+ if (_enabledServerInstanceMap.put(instanceId, serverInstance) ==
null) {
+ newEnabledServers.add(instanceId);
+
+ // NOTE: Remove new enabled server from excluded servers because
the server is likely being restarted
+ if (_excludedServers.remove(instanceId)) {
+ LOGGER.info("Got excluded server: {} re-enabled, including it
into the routing", instanceId);
+ }
}
}
+ } catch (Exception e) {
+ LOGGER.error("Cannot add server instance {}, ignored it, due to
error", instanceId, e);
Review Comment:
```suggestion
LOGGER.error("Caught exception while adding instance: {}, ignoring
it", instanceId, e);
```
--
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]
