From 86898beb8792bcb379267f00127a8d09387039a9 Mon Sep 17 00:00:00 2001 From: Mingqiang Liang Date: Tue, 26 Nov 2024 15:12:35 -0800 Subject: [PATCH] refactor code by adding ServerInfoCache --- .../helix/SegmentStatusChecker.java | 32 ++++--- .../helix/core/PinotHelixResourceManager.java | 5 -- .../controller/util/ServerInfoCache.java | 83 +++++++++++++++++++ .../helix/SegmentStatusCheckerTest.java | 63 +++----------- 4 files changed, 109 insertions(+), 74 deletions(-) create mode 100644 pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerInfoCache.java diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java index 41a39efb7baa..117f4b2a38e4 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java @@ -29,7 +29,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.helix.model.ExternalView; import org.apache.helix.model.IdealState; -import org.apache.helix.model.InstanceConfig; import org.apache.helix.store.zk.ZkHelixPropertyStore; import org.apache.helix.zookeeper.datamodel.ZNRecord; import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer; @@ -48,6 +47,8 @@ import org.apache.pinot.controller.helix.core.periodictask.ControllerPeriodicTask; import org.apache.pinot.controller.helix.core.realtime.MissingConsumingSegmentFinder; import org.apache.pinot.controller.helix.core.realtime.PinotLLCRealtimeSegmentManager; +import org.apache.pinot.controller.util.ServerInfoCache; +import org.apache.pinot.controller.util.ServerInfoCache.ServerInfo; import org.apache.pinot.controller.util.TableSizeReader; import org.apache.pinot.spi.config.table.TableConfig; import org.apache.pinot.spi.config.table.TableType; @@ -82,6 +83,8 @@ public class SegmentStatusChecker extends ControllerPeriodicTask entry : stateMap.entrySet()) { String serverInstanceId = entry.getKey(); String state = entry.getValue(); - if (context._queryableServers.contains(serverInstanceId) - && (state.equals(SegmentStateModel.ONLINE) || state.equals(SegmentStateModel.CONSUMING))) { + if (isServerQueryable(serverInstanceId) + && (state.equals(SegmentStateModel.ONLINE) || state.equals(SegmentStateModel.CONSUMING))) { numEVReplicasUp++; } if (state.equals(SegmentStateModel.ERROR)) { @@ -444,6 +435,14 @@ private void updateSegmentMetrics(String tableNameWithType, TableConfig tableCon } } + private boolean isServerQueryable(String serverInstanceId) { + ServerInfo serverInfo = _serverInfoCache.getServerInfo(serverInstanceId); + return serverInfo != null + && serverInfo.helixEnabled + && !serverInfo.queriesDisabled + && !serverInfo.shutdownInProgress; + } + private static String logSegments(List segments) { if (segments.size() <= MAX_SEGMENTS_TO_LOG) { return segments.toString(); @@ -491,6 +490,5 @@ public static final class Context { private final Set _processedTables = new HashSet<>(); private final Set _disabledTables = new HashSet<>(); private final Set _pausedTables = new HashSet<>(); - private final Set _queryableServers = new HashSet<>(); } } diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java index f009cbddc9ae..e7affa4287d6 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java @@ -515,11 +515,6 @@ public List getAllControllerInstanceConfigs() { .filter(instance -> InstanceTypeUtils.isController(instance.getId())).collect(Collectors.toList()); } - public List getAllServerInstanceConfigs() { - return HelixHelper.getInstanceConfigs(_helixZkManager).stream() - .filter(instance -> InstanceTypeUtils.isServer(instance.getId())).collect(Collectors.toList()); - } - public List getAllMinionInstanceConfigs() { return HelixHelper.getInstanceConfigs(_helixZkManager).stream() .filter(instance -> InstanceTypeUtils.isMinion(instance.getId())).collect(Collectors.toList()); diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerInfoCache.java b/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerInfoCache.java new file mode 100644 index 000000000000..a7e907bf0363 --- /dev/null +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerInfoCache.java @@ -0,0 +1,83 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.controller.util; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.helix.model.InstanceConfig; +import org.apache.helix.zookeeper.datamodel.ZNRecord; +import org.apache.pinot.controller.helix.core.PinotHelixResourceManager; +import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.InstanceTypeUtils; + + +/** + * This is a helper class maintaining a cache of server information. It is used to avoid repeated calls to Helix to + * get server information. This class is not thread safe. + */ +public class ServerInfoCache { + PinotHelixResourceManager pinotHelixResourceManager; + Map serverInfoMap; + + public ServerInfoCache(PinotHelixResourceManager pinotHelixResourceManager) { + this.pinotHelixResourceManager = pinotHelixResourceManager; + this.serverInfoMap = new HashMap<>(); + } + + public ServerInfo getServerInfo(String instanceId) { + return serverInfoMap.computeIfAbsent(instanceId, this::getServerInfoOndemand); + } + + private ServerInfo getServerInfoOndemand(String instanceId) { + InstanceConfig instanceConfig = pinotHelixResourceManager.getHelixInstanceConfig(instanceId); + if (instanceConfig == null || !InstanceTypeUtils.isServer(instanceId)) { + return null; + } + List tags = instanceConfig.getTags(); + ZNRecord record = instanceConfig.getRecord(); + boolean helixEnabled = record.getBooleanField( + InstanceConfig.InstanceConfigProperty.HELIX_ENABLED.name(), false); + boolean queriesDisabled = record.getBooleanField(CommonConstants.Helix.QUERIES_DISABLED, false); + boolean shutdownInProgress = record.getBooleanField(CommonConstants.Helix.IS_SHUTDOWN_IN_PROGRESS, false); + return new ServerInfo(instanceId, tags, null, helixEnabled, queriesDisabled, shutdownInProgress); + } + + public static class ServerInfo { + public String instanceName; + public List tags; + public List tables; + public boolean helixEnabled; + public boolean queriesDisabled; + public boolean shutdownInProgress; + public ServerInfo(String instanceName, + List tags, + List tables, + boolean helixEnabled, + boolean queriesDisabled, + boolean shutdownInProgress) { + this.instanceName = instanceName; + this.tags = tags; + this.tables = tables; + this.helixEnabled = helixEnabled; + this.queriesDisabled = queriesDisabled; + this.shutdownInProgress = shutdownInProgress; + } + } +} diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java index a6c3df63be0d..547f45bf8074 100644 --- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java +++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/SegmentStatusCheckerTest.java @@ -107,13 +107,7 @@ public void offlineBasicTest() { externalView.setState("myTable_4", "pinot1", "ONLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQuerableInstanceConfig("pinot1"), - newQuerableInstanceConfig("pinot2"), - newQuerableInstanceConfig("pinot3") - ) - ); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getAllTables()).thenReturn(List.of(OFFLINE_TABLE_NAME)); when(resourceManager.getTableConfig(OFFLINE_TABLE_NAME)).thenReturn(tableConfig); when(resourceManager.getTableIdealState(OFFLINE_TABLE_NAME)).thenReturn(idealState); @@ -224,12 +218,7 @@ public void realtimeBasicTest() { externalView.setState(seg3, "pinot3", "OFFLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQuerableInstanceConfig("pinot1"), - newQuerableInstanceConfig("pinot2"), - newQuerableInstanceConfig("pinot3")) - ); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getTableConfig(REALTIME_TABLE_NAME)).thenReturn(tableConfig); when(resourceManager.getAllTables()).thenReturn(List.of(REALTIME_TABLE_NAME)); when(resourceManager.getTableIdealState(REALTIME_TABLE_NAME)).thenReturn(idealState); @@ -295,14 +284,7 @@ public void realtimeMutableSegmentHasLessReplicaTest() { externalView.setState(seg3, "pinot4", "OFFLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQuerableInstanceConfig("pinot1"), - newQuerableInstanceConfig("pinot2"), - newQuerableInstanceConfig("pinot3"), - newQuerableInstanceConfig("pinot4") - ) - ); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getTableConfig(REALTIME_TABLE_NAME)).thenReturn(tableConfig); when(resourceManager.getAllTables()).thenReturn(List.of(REALTIME_TABLE_NAME)); when(resourceManager.getTableIdealState(REALTIME_TABLE_NAME)).thenReturn(idealState); @@ -368,14 +350,10 @@ public void realtimeServerNotQueryableTest() { externalView.setState(seg3, "Server_pinot4", "OFFLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQueryDisabledInstanceConfig("Server_pinot1"), - newShutdownInProgressInstanceConfig("Server_pinot2"), - newQuerableInstanceConfig("Server_pinot3"), - newQuerableInstanceConfig("Server_pinot4") - ) - ); + when(resourceManager.getHelixInstanceConfig("Server_pinot1")).thenReturn(newQueryDisabledInstanceConfig("Server_pinot1")); + when(resourceManager.getHelixInstanceConfig("Server_pinot2")).thenReturn(newShutdownInProgressInstanceConfig("Server_pinot2")); + when(resourceManager.getHelixInstanceConfig("Server_pinot3")).thenReturn(newQuerableInstanceConfig("Server_pinot3")); + when(resourceManager.getHelixInstanceConfig("Server_pinot4")).thenReturn(newQuerableInstanceConfig("Server_pinot4")); when(resourceManager.getTableConfig(REALTIME_TABLE_NAME)).thenReturn(tableConfig); when(resourceManager.getAllTables()).thenReturn(List.of(REALTIME_TABLE_NAME)); when(resourceManager.getTableIdealState(REALTIME_TABLE_NAME)).thenReturn(idealState); @@ -461,14 +439,7 @@ public void realtimeImmutableSegmentHasLessReplicaTest() { externalView.setState(seg3, "pinot4", "OFFLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQuerableInstanceConfig("pinot1"), - newQuerableInstanceConfig("pinot2"), - newQuerableInstanceConfig("pinot3"), - newQuerableInstanceConfig("pinot4") - ) - ); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getTableConfig(REALTIME_TABLE_NAME)).thenReturn(tableConfig); when(resourceManager.getAllTables()).thenReturn(List.of(REALTIME_TABLE_NAME)); when(resourceManager.getTableIdealState(REALTIME_TABLE_NAME)).thenReturn(idealState); @@ -535,13 +506,7 @@ public void missingEVPartitionTest() { externalView.setState("myTable_1", "pinot2", "ONLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQuerableInstanceConfig("pinot1"), - newQuerableInstanceConfig("pinot2"), - newQuerableInstanceConfig("pinot3") - ) - ); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getAllTables()).thenReturn(List.of(OFFLINE_TABLE_NAME)); when(resourceManager.getTableIdealState(OFFLINE_TABLE_NAME)).thenReturn(idealState); when(resourceManager.getTableExternalView(OFFLINE_TABLE_NAME)).thenReturn(externalView); @@ -632,12 +597,7 @@ public void missingEVPartitionPushTest() { externalView.setState("myTable_2", "pinot1", "ONLINE"); PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of( - newQuerableInstanceConfig("pinot1"), - newQuerableInstanceConfig("pinot2") - ) - ); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getAllTables()).thenReturn(List.of(OFFLINE_TABLE_NAME)); when(resourceManager.getTableIdealState(OFFLINE_TABLE_NAME)).thenReturn(idealState); when(resourceManager.getTableExternalView(OFFLINE_TABLE_NAME)).thenReturn(externalView); @@ -780,8 +740,7 @@ public void lessThanOnePercentSegmentsUnavailableTest() { } PinotHelixResourceManager resourceManager = mock(PinotHelixResourceManager.class); - when(resourceManager.getAllServerInstanceConfigs()).thenReturn( - List.of(newQuerableInstanceConfig("pinot1"))); + when(resourceManager.getHelixInstanceConfig(any())).thenReturn(newQuerableInstanceConfig("any")); when(resourceManager.getAllTables()).thenReturn(List.of(OFFLINE_TABLE_NAME)); when(resourceManager.getTableConfig(OFFLINE_TABLE_NAME)).thenReturn(tableConfig); when(resourceManager.getTableIdealState(OFFLINE_TABLE_NAME)).thenReturn(idealState);