gortiz commented on code in PR #19568:
URL: https://github.com/apache/pinot/pull/19568#discussion_r4063612448
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java:
##########
@@ -633,8 +641,9 @@ private <E> void execute(long requestId,
Set<DispatchablePlanFragment> stagePlan
ByteString protoRequestMetadata =
QueryPlanSerDeUtils.toProtoProperties(requestMetadata);
// Submit the query plan to all servers in parallel
+ boolean protoSegmentList =
QueryOptionsUtils.isProtoSegmentList(queryOptions,
_protoSegmentList.isEnabled());
BlockingQueue<AsyncResponse<E>> dispatchCallbacks = dispatch(sendRequest,
serverInstancesOut, deadline,
- serverInstance -> createRequest(serverInstance, stageInfos,
protoRequestMetadata));
+ serverInstance -> createRequest(serverInstance, stageInfos,
protoRequestMetadata, protoSegmentList));
Review Comment:
The encode moved into the dispatch loop, not just off the compile executor,
and I think that costs tail latency on the default path.
`dispatch()` just below runs `R request =
requestBuilder.apply(serverInstance); ... sendRequest.send(...)` in a single
loop on the calling thread. Before this PR the segment lists were already
encoded at plan time, so this was a tight send loop. Now every iteration does
that server's encode first.
With the numbers from the description -- 60k segments at ~16ms for the
legacy JSON encode, which is the **default** path -- the last server in a wide
fan-out starts its leaf stage materially later than the first, purely because
encode time is interleaved with sends. Total CPU is unchanged, but the query
finishes when the slowest server finishes.
Easy to keep: build all the requests in one pass over `serverInstancesOut`,
then run the send loop. That preserves the "off the compile executor" win,
which is the actual motivation, without introducing dispatch skew. It also
makes the benchmark table true for the default configuration rather than only
for the opt-in one.
Same pattern in `submitWithStream` (around line 369), which also builds and
sends in one loop.
Worth measuring on a wide fan-out against a large table before and after --
the benchmark in the description is a single leaf worker, so it cannot show
this either way.
(Unrelated, and good: reading the flag once per request rather than per
server is the right granularity -- all servers of one query agree on the
encoding even if an operator flips the cluster config mid-dispatch.)
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/ProtoSegmentListPredicate.java:
##########
@@ -0,0 +1,109 @@
+/**
+ * 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.query.service.dispatch;
+
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.concurrent.ThreadSafe;
+import org.apache.pinot.spi.config.provider.PinotClusterConfigChangeListener;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/// The cluster-level default for encoding leaf-stage segment lists as native
protobuf fields of the worker metadata,
+/// read by [QueryDispatcher] on every request that does not carry an explicit
+/// [CommonConstants.Broker.Request.QueryOptionKey#PROTO_SEGMENT_LIST]
override.
+///
+/// The value is seeded from the static broker configuration and can then be
changed through cluster config, on the
+/// same key, without restarting the brokers. That matters because the setting
is only safe once every server of the
+/// cluster understands the proto fields: an older server finds no segments
under them, concludes the worker is not a
+/// leaf-stage worker and fails its leaf stage. Operators therefore want to
turn it on at the exact moment a rolling
+/// upgrade completes, and to turn it back off immediately if it misbehaves,
neither of which should cost a broker
+/// restart. Cluster config wins over the static seed because
[org.apache.pinot.common.config
+/// .DefaultClusterConfigChangeHandler] replays the current cluster config to
a listener as soon as it is registered;
+/// clearing the key from cluster config falls back to
[CommonConstants.Broker#DEFAULT_MSE_PROTO_SEGMENT_LIST], not to
+/// the static seed.
+///
+/// Thread-safety: `_enabled` is `volatile`, so [#isEnabled()] stays lock-free
on the request path. [#onChange] is
+/// `synchronized` only so that the `previous -> new` pair in its log line
cannot interleave with another delivery. It
+/// does *not* order deliveries: the change handler invokes listeners outside
its own lock, so a delivery computed
+/// from an older snapshot can still be applied after a newer one and leave a
stale value until the next
+/// cluster-config change.
+@ThreadSafe
+public class ProtoSegmentListPredicate implements
PinotClusterConfigChangeListener {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ProtoSegmentListPredicate.class);
+ private static final String KEY =
CommonConstants.Broker.CONFIG_OF_MSE_PROTO_SEGMENT_LIST;
+ private static final String ENABLE_WARNING =
+ "Every server of the cluster must already run a version that understands
the proto segment list fields. "
+ + "Leaf stages routed to an older server will fail. Set it back to
false to revert.";
+
+ private volatile boolean _enabled;
+
+ public ProtoSegmentListPredicate(boolean enabled) {
+ _enabled = enabled;
+ }
+
+ /// Seeds the value from the static broker configuration. NOTE: the Helix
manager is not necessarily connected when
+ /// this is called, so a cluster-config override is applied later through
[#onChange].
+ public static ProtoSegmentListPredicate create(PinotConfiguration
brokerConf) {
Review Comment:
There is prior art for this in the same module, and I think it removes most
of this PR's operational surface.
`pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/SendStatsPredicate.java`
handles the same class of problem -- MSE sends something a pre-1.4 peer cannot
handle -- and given the shared `*Predicate` name I assume it is where this
class came from. But it took the opposite half of the design:
> `SendStatsPredicate.Safe` implements `InstanceConfigChangeListener`, reads
`CommonConstants.Helix.Instance.PINOT_VERSION_KEY` off each instance's
`InstanceConfig` (line 228), and keeps `_problematicVersionsById`. It enables
itself the moment the last old instance disappears. Modes are `ALWAYS` / `SAFE`
/ `NEVER`, and `SAFE` needs no human.
The property is already there and already published:
`HelixHelper.updatePinotVersion` is called from `BaseServerStarter` (line 529),
`BaseBrokerStarter` (line 889), `BaseControllerStarter` and
`BaseMinionStarter`. Nothing new has to be written or watched.
Shape of the change, reusing this config key rather than adding one:
```java
// CommonConstants.Broker
CONFIG_OF_MSE_PROTO_SEGMENT_LIST = "pinot.broker.mse.proto.segment.list" //
unchanged
DEFAULT_MSE_PROTO_SEGMENT_LIST = "SAFE" //
was: boolean false
enum Mode { NEVER, SAFE, ALWAYS }
NEVER - always legacy JSON. The kill switch.
SAFE - proto while no server reports a version older than the one that
added
fields 4/5. Default. Turns itself on when the rollout finishes.
ALWAYS - proto unconditionally. Escape hatch for clusters that do not
publish
versions, for forks carrying the change on a different version
string,
and for tests.
```
Registration gets simpler, not harder: make this a `ClusterChangeHandler`
and add it to `_instanceConfigChangeHandlers` in `BaseBrokerStarter` (line
778), next to `_routingManager` and `_queryQuotaManager`. That is an existing
extension point fed by `_clusterChangeMediator` on
`ChangeType.INSTANCE_CONFIG`, so the bespoke registration added in this PR, the
comment defending its position, and
`testRegistrationBeforeFirstDeliveryStillLetsClusterConfigWin` all go away.
What it removes: the four-step enablement runbook, the "verify the version
of every `Server_*` instance" precondition, the canary query, the two curl
commands, the rollback procedure, and the window in which a mis-timed flip
silently turns leaf workers into intermediate workers. The per-query
`protoSegmentList` option stays as the override. It is also more responsive
than a cluster config, because it reacts to the rollout itself rather than to
an operator noticing the rollout finished.
Two honest weaknesses, both survivable:
1. `SendStatsPredicate.isProblematicVersion()` treats any version `!=
PinotVersion.VERSION` as problematic, including a newer one, so a heterogeneous
cluster stays on the legacy encoding. Strictly correct here would be "server
version >= the release that added fields 4/5", which means comparing Pinot
version strings (`1.4.0-SNAPSHOT`, `UNKNOWN`, fork versions) -- parsing nobody
wants to own. I would copy the conservative equality: it is over-cautious, it
self-heals exactly when the fleet becomes homogeneous, and that is precisely
the moment this PR's runbook tells a human to flip the flag. `ALWAYS` covers
whoever cannot wait.
2. A missing or `UNKNOWN` version must read as old, i.e. legacy encoding.
Fail closed, same as the unparseable-boolean handling below, which is already
right.
Most of `ProtoSegmentListPredicateTest` survives in shape -- swap the
`DefaultClusterConfigChangeHandler` deliveries for instance-config ones.
Was this approach considered and rejected? If so it would be worth saying
why in the description.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/QueryPlanSerDeUtils.java:
##########
@@ -54,15 +73,55 @@ private static StageMetadata
fromProtoStageMetadata(Worker.StageMetadata protoSt
return new StageMetadata(protoStageMetadata.getStageId(),
workerMetadataList, customProperties);
}
- private static WorkerMetadata fromProtoWorkerMetadata(Worker.WorkerMetadata
protoWorkerMetadata)
+ @VisibleForTesting
+ static WorkerMetadata fromProtoWorkerMetadata(Worker.WorkerMetadata
protoWorkerMetadata)
throws InvalidProtocolBufferException {
Map<Integer, ByteString> protoMailboxInfosMap =
protoWorkerMetadata.getMailboxInfosMap();
Map<Integer, MailboxInfos> mailboxInfosMap =
Maps.newHashMapWithExpectedSize(protoMailboxInfosMap.size());
for (Map.Entry<Integer, ByteString> entry :
protoMailboxInfosMap.entrySet()) {
mailboxInfosMap.put(entry.getKey(),
fromProtoMailboxInfos(entry.getValue()));
}
- return new WorkerMetadata(protoWorkerMetadata.getWorkedId(),
mailboxInfosMap,
- protoWorkerMetadata.getCustomPropertyMap());
+ // A broker using the legacy encoding ships the segment maps as JSON
custom properties. Decode them once here and
+ // drop the raw strings so that the metadata never carries two copies of
the same segments.
+ Map<String, String> customProperties =
protoWorkerMetadata.getCustomPropertyMap();
+ String tableSegmentsJson =
customProperties.get(WorkerMetadata.TABLE_SEGMENTS_MAP_KEY);
+ String logicalTableSegmentsJson =
customProperties.get(WorkerMetadata.LOGICAL_TABLE_SEGMENTS_MAP_KEY);
+ if (tableSegmentsJson != null || logicalTableSegmentsJson != null) {
+ customProperties = new HashMap<>(customProperties);
+ customProperties.remove(WorkerMetadata.TABLE_SEGMENTS_MAP_KEY);
+ customProperties.remove(WorkerMetadata.LOGICAL_TABLE_SEGMENTS_MAP_KEY);
+ }
+ WorkerMetadata workerMetadata =
+ new WorkerMetadata(protoWorkerMetadata.getWorkedId(), mailboxInfosMap,
customProperties);
+ if (protoWorkerMetadata.hasTableSegmentsMap()) {
Review Comment:
Decoding proto first and JSON second is the right direction and is what
makes the rollout order broker-agnostic. Only the reverse case -- old server,
new broker with the encoding on -- breaks, and that is the documented
precondition.
One thing that would make that case much less painful, and is worth doing
whatever happens to the enablement mechanism: when neither
`hasTableSegmentsMap()` nor the JSON key is present, this worker is treated as
an intermediate-stage worker. That is exactly what an old server does when it
receives the new encoding, and it is silent -- a leaf stage that scans nothing,
not an error.
Since the server can see the stage plan, a `WARN` when a stage plan whose
root contains a `TableScanNode` arrives with no segment map would turn the one
dangerous misconfiguration into something greppable, at no cost on the happy
path.
##########
pinot-query-planner/src/test/java/org/apache/pinot/query/routing/QueryPlanSerDeUtilsTest.java:
##########
@@ -0,0 +1,173 @@
+/**
+ * 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.query.routing;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.apache.pinot.common.proto.Worker;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
+
+/// Tests the two wire encodings of the leaf-stage segment maps in
[QueryPlanSerDeUtils]: the native proto fields and
+/// the legacy JSON custom properties, including a server decoding what a
pre-proto broker sends.
+public class QueryPlanSerDeUtilsTest {
+ private static final Map<String, String> CUSTOM_PROPERTIES = Map.of("foo",
"bar");
+ private static final Map<String, List<String>> TABLE_SEGMENTS_MAP =
+ Map.of("OFFLINE", List.of("seg_0", "seg_1"), "REALTIME",
List.of("seg__0__0__20240101T0000Z"));
+ private static final Map<String, List<String>> LOGICAL_TABLE_SEGMENTS_MAP =
+ Map.of("t1_OFFLINE", List.of("t1_seg_0"), "t2_REALTIME",
List.of("t2_seg_0", "t2_seg_1"));
+
+ @DataProvider
+ public static Object[][] encodings() {
+ return new Object[][]{{true}, {false}};
+ }
+
+ @Test(dataProvider = "encodings")
+ public void testLeafWorkerRoundTrip(boolean protoSegmentList)
+ throws Exception {
+ WorkerMetadata workerMetadata = leafWorker(TABLE_SEGMENTS_MAP, null);
+
+ Worker.WorkerMetadata proto = toProto(workerMetadata, protoSegmentList);
+ assertEquals(proto.hasTableSegmentsMap(), protoSegmentList);
+ assertFalse(proto.hasLogicalTableSegmentsMap());
+
assertEquals(proto.getCustomPropertyMap().containsKey(WorkerMetadata.TABLE_SEGMENTS_MAP_KEY),
!protoSegmentList);
+
assertFalse(proto.getCustomPropertyMap().containsKey(WorkerMetadata.LOGICAL_TABLE_SEGMENTS_MAP_KEY));
+ assertEquals(proto.getCustomPropertyMap().get("foo"), "bar");
+
+ WorkerMetadata decoded =
QueryPlanSerDeUtils.fromProtoWorkerMetadata(proto);
+ assertEquals(decoded.getWorkerId(), 3);
+ assertEquals(decoded.getTableSegmentsMap(), TABLE_SEGMENTS_MAP);
+ assertNull(decoded.getLogicalTableSegmentsMap());
+ assertTrue(decoded.isLeafStageWorker());
+ // The JSON is never surfaced as a custom property of the decoded
metadata, whichever encoding was used.
+ assertEquals(decoded.getCustomProperties(), CUSTOM_PROPERTIES);
+ MailboxInfo mailboxInfo =
decoded.getMailboxInfosMap().get(2).getMailboxInfos().get(0);
+ assertEquals(mailboxInfo.getHostname(), "localhost");
+ assertEquals(mailboxInfo.getPort(), 1234);
+ assertEquals(mailboxInfo.getWorkerIds(), List.of(0, 1));
+ }
+
+ @Test(dataProvider = "encodings")
+ public void testLogicalTableLeafWorkerRoundTrip(boolean protoSegmentList)
+ throws Exception {
+ WorkerMetadata workerMetadata = leafWorker(null,
LOGICAL_TABLE_SEGMENTS_MAP);
+
+ Worker.WorkerMetadata proto = toProto(workerMetadata, protoSegmentList);
+ assertFalse(proto.hasTableSegmentsMap());
+ assertEquals(proto.hasLogicalTableSegmentsMap(), protoSegmentList);
+
assertEquals(proto.getCustomPropertyMap().containsKey(WorkerMetadata.LOGICAL_TABLE_SEGMENTS_MAP_KEY),
+ !protoSegmentList);
+
+ WorkerMetadata decoded =
QueryPlanSerDeUtils.fromProtoWorkerMetadata(proto);
+ assertNull(decoded.getTableSegmentsMap());
+ assertEquals(decoded.getLogicalTableSegmentsMap(),
LOGICAL_TABLE_SEGMENTS_MAP);
+ assertTrue(decoded.isLeafStageWorker());
+ assertEquals(decoded.getCustomProperties(), CUSTOM_PROPERTIES);
+ }
+
+ @Test(dataProvider = "encodings")
+ public void testEmptySegmentListStillMarksLeafWorker(boolean
protoSegmentList)
+ throws Exception {
+ // A padded worker of a partitioned table scans no segment but must still
run the leaf stage.
+ Map<String, List<String>> emptySegments = Map.of("OFFLINE", new
ArrayList<>());
Review Comment:
This case does not quite test what its name suggests: `Map.of("OFFLINE", new
ArrayList<>())` is a *non-empty* `SegmentsMap` holding an empty `SegmentList`.
The presence bit keeps `isLeafStageWorker()` true either way, so the test
passes, but it does not exercise the interesting shape: a segments map with
zero entries, which encodes to `SegmentsMap.getDefaultInstance()`. That is
where proto3's explicit presence for singular message fields is doing the work,
and it is the one case where a naive reading would expect the field to vanish
from the wire entirely.
Since the whole leaf/intermediate distinction now rides on that presence
bit, a `Map.of()` case is worth adding to pin the semantics.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/QueryPlanSerDeUtils.java:
##########
@@ -54,15 +73,55 @@ private static StageMetadata
fromProtoStageMetadata(Worker.StageMetadata protoSt
return new StageMetadata(protoStageMetadata.getStageId(),
workerMetadataList, customProperties);
}
- private static WorkerMetadata fromProtoWorkerMetadata(Worker.WorkerMetadata
protoWorkerMetadata)
+ @VisibleForTesting
+ static WorkerMetadata fromProtoWorkerMetadata(Worker.WorkerMetadata
protoWorkerMetadata)
throws InvalidProtocolBufferException {
Map<Integer, ByteString> protoMailboxInfosMap =
protoWorkerMetadata.getMailboxInfosMap();
Map<Integer, MailboxInfos> mailboxInfosMap =
Maps.newHashMapWithExpectedSize(protoMailboxInfosMap.size());
for (Map.Entry<Integer, ByteString> entry :
protoMailboxInfosMap.entrySet()) {
mailboxInfosMap.put(entry.getKey(),
fromProtoMailboxInfos(entry.getValue()));
}
- return new WorkerMetadata(protoWorkerMetadata.getWorkedId(),
mailboxInfosMap,
- protoWorkerMetadata.getCustomPropertyMap());
+ // A broker using the legacy encoding ships the segment maps as JSON
custom properties. Decode them once here and
+ // drop the raw strings so that the metadata never carries two copies of
the same segments.
+ Map<String, String> customProperties =
protoWorkerMetadata.getCustomPropertyMap();
+ String tableSegmentsJson =
customProperties.get(WorkerMetadata.TABLE_SEGMENTS_MAP_KEY);
+ String logicalTableSegmentsJson =
customProperties.get(WorkerMetadata.LOGICAL_TABLE_SEGMENTS_MAP_KEY);
+ if (tableSegmentsJson != null || logicalTableSegmentsJson != null) {
+ customProperties = new HashMap<>(customProperties);
Review Comment:
The custom-property map handed to `WorkerMetadata` ends up mutable or
immutable depending on which encoding the broker used.
When either JSON key is present it becomes a `HashMap`; otherwise it stays
the proto's immutable map view. So on a server, whether
`workerMetadata.getCustomProperties().put(...)` throws depends on the encoding
of the request that produced it -- a difference that would only ever surface in
production, under the new encoding, on whatever path decides to write to it.
Either always copy, or always wrap in an unmodifiable view. The copy is
cheap next to what this method already does.
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/service/server/QueryServerTest.java:
##########
@@ -235,13 +235,26 @@ public void onCompleted() {
@Test(dataProvider = "testSql")
public void testWorkerAcceptsWorkerRequestCorrect(String sql)
throws Exception {
+ testWorkerAcceptsWorkerRequestCorrect(sql, false);
+ }
+
+ /// Same as [#testWorkerAcceptsWorkerRequestCorrect(String)] with the
leaf-stage segment lists shipped as native
+ /// proto fields instead of the legacy JSON custom property.
+ @Test(dataProvider = "testSql")
+ public void testWorkerAcceptsProtoSegmentListRequestCorrect(String sql)
Review Comment:
Good to have both encodings covered here, but note what this still does not
reach: it builds the request by calling `toProtoWorkerMetadataList` directly,
so it covers serde plus server decode while bypassing `QueryDispatcher`, the
query option and the live config -- that is, everything that decides *which*
encoding is used. The two halves the rollout depends on are never tested
together.
For a change gated on an operational procedure I would want one integration
test running a real query with `SET protoSegmentList = true` and asserting the
same result as without it, plus the same for a logical table, since
`logicalTableSegmentsMap` is keyed differently and only
`ServerPlanRequestUtils.constructLogicalTableServerQueryRequests` reads it.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/physical/DispatchablePlanContext.java:
##########
@@ -198,11 +198,19 @@ public Map<Integer, DispatchablePlanFragment>
constructDispatchablePlanFragmentM
QueryServerInstance queryServerInstance = serverEntry.getValue();
serverInstanceToWorkerIdsMap.computeIfAbsent(queryServerInstance, k ->
new ArrayList<>()).add(workerId);
WorkerMetadata workerMetadata = new WorkerMetadata(workerId,
workerIdToMailboxesMap.get(workerId));
+ // A leaf-stage worker is identified by carrying a (possibly empty)
segment map, so every worker of a
+ // scanning stage has to be present in the map. Fail loudly here
instead of letting the worker decay
+ // into an intermediate-stage worker on the server.
if (workerIdToSegmentsMap != null) {
-
workerMetadata.setTableSegmentsMap(workerIdToSegmentsMap.get(workerId));
+ Map<String, List<String>> segmentsMap =
workerIdToSegmentsMap.get(workerId);
+ Preconditions.checkNotNull(segmentsMap, "Missing segments map for
worker id: %s", workerId);
Review Comment:
This is a behavior change independent of the encoding work, and I think it
should be called out in the description (and ideally be its own commit).
Before this PR a null here went to `JsonUtils.objectToString(null)`, i.e.
the literal string `"null"` as the custom property, which made
`isLeafStageWorker()` true while `getTableSegmentsMap()` returned null. On the
server that lands in `ServerPlanRequestUtils.constructServerQueryRequests`,
whose only guard is `assert tableSegmentsMap != null` -- disabled in production
-- so it became an NPE deep in leaf-stage compilation. Failing at plan time
with the worker id is strictly better, so no objection to the change itself.
I checked the sites that populate `workerIdToSegmentsMap` (`WorkerManager`
around lines 449, 668, 959 and 1436, plus `PlanFragmentAndMailboxAssignment` in
the V2 planner) and they all iterate `workerIdToServerInstanceMap`'s key set,
so every worker gets an entry and this should never fire. That reassurance is
worth writing down somewhere more durable than a review thread.
The reason to split it: a bisect of a segment-map failure should not land on
"switch to proto encoding".
--
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]
