61yao commented on code in PR #10350:
URL: https://github.com/apache/pinot/pull/10350#discussion_r1137544402
##########
pinot-broker/src/test/java/org/apache/pinot/broker/routing/instanceselector/InstanceSelectorTest.java:
##########
@@ -106,12 +139,13 @@ public void testInstanceSelector() {
String offlineTableName = "testTable_OFFLINE";
BrokerMetrics brokerMetrics = mock(BrokerMetrics.class);
AdaptiveServerSelector adaptiveServerSelector = null;
- BalancedInstanceSelector balancedInstanceSelector = new
BalancedInstanceSelector(offlineTableName, brokerMetrics,
- adaptiveServerSelector);
+ ZkHelixPropertyStore<ZNRecord> propertyStore =
mock(ZkHelixPropertyStore.class);
+ BalancedInstanceSelector balancedInstanceSelector =
Review Comment:
done
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/StrictReplicaGroupInstanceSelector.java:
##########
@@ -58,13 +62,28 @@
* transitioning/error scenario (external view does not match ideal state), if
a segment is down on S1, we mark all
* segments with the same assignment ([S1, S2, S3]) down on S1 to ensure that
we always route the segments to the same
* replica-group.
- * </pre>
+ *
+ * Note that New segment won't be used to exclude instance from serving where
new segment is unavailable.
+ *
+ * </pre>
*/
public class StrictReplicaGroupInstanceSelector extends
ReplicaGroupInstanceSelector {
- public StrictReplicaGroupInstanceSelector(String tableNameWithType,
BrokerMetrics brokerMetrics, @Nullable
- AdaptiveServerSelector adaptiveServerSelector) {
- super(tableNameWithType, brokerMetrics, adaptiveServerSelector);
+ private boolean isNewSegment(String segment, Map<String, SegmentState>
newSegmentStateMap, long nowMillis) {
+ SegmentState state = newSegmentStateMap.get(segment);
+ return state != null && state.isNew(nowMillis);
+ }
+
+ public StrictReplicaGroupInstanceSelector(String tableNameWithType,
BrokerMetrics brokerMetrics,
+ @Nullable AdaptiveServerSelector adaptiveServerSelector,
ZkHelixPropertyStore<ZNRecord> propertyStore) {
+ this(tableNameWithType, brokerMetrics, adaptiveServerSelector,
propertyStore, Clock.systemUTC());
+ }
+
+ // Test only for clock injection.
Review Comment:
removed
--
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]
