61yao commented on code in PR #10350: URL: https://github.com/apache/pinot/pull/10350#discussion_r1144063961
########## pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/BaseInstanceSelector.java: ########## @@ -18,69 +18,162 @@ */ package org.apache.pinot.broker.routing.instanceselector; +import java.time.Clock; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.SortedMap; import javax.annotation.Nullable; +import org.apache.helix.AccessOption; import org.apache.helix.model.ExternalView; import org.apache.helix.model.IdealState; +import org.apache.helix.store.zk.ZkHelixPropertyStore; +import org.apache.helix.zookeeper.datamodel.ZNRecord; import org.apache.pinot.broker.routing.adaptiveserverselector.AdaptiveServerSelector; import org.apache.pinot.broker.routing.segmentpreselector.SegmentPreSelector; -import org.apache.pinot.common.metrics.BrokerMeter; +import org.apache.pinot.common.metadata.ZKMetadataProvider; +import org.apache.pinot.common.metadata.segment.SegmentZKMetadata; import org.apache.pinot.common.metrics.BrokerMetrics; import org.apache.pinot.common.request.BrokerRequest; -import org.apache.pinot.common.utils.HashUtil; import org.apache.pinot.spi.utils.CommonConstants.Helix.StateModel.SegmentStateModel; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** - * Base implementation of instance selector which maintains a map from segment to enabled ONLINE/CONSUMING server + * Base implementation of instance selector. Selector maintains a map from segment to enabled ONLINE/CONSUMING server * instances that serves the segment and a set of unavailable segments (no enabled instance or all enabled instances are * in ERROR state). + * <p> + * Special handling of new segment: It is common for new segment to be partially available or not available at all in + * all instances. + * 1) We don't report new segment as unavailable segments. + * 2) To increase query availability, unavailable + * instance for new segment won't be excluded for instance selection. When it is selected, we don't serve the new + * segment. + * <p> + * Definition of new segment: + * 1) Segment created more than 5 minutes ago. + * - If we first see a segment via initialization, we look up segment creation time from zookeeper. + * - If we first see a segment via onAssignmentChange initialization, we use the calling time of onAssignmentChange + * as approximation. + * 2) We retire new segment as old when: + * - The creation time is more than 5 minutes ago + * - Any instance for new segment is in error state + * - External view for segment converges with ideal state. + * + * Note that this implementation means: + * 1) Inconsistent selection of new segments across queries. (some queries will serve new segments and others won't) + * 2) When there is no state update from helix, new segments won't be retired because of the time passing. Review Comment: done. -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
