egalpin commented on code in PR #13742: URL: https://github.com/apache/pinot/pull/13742#discussion_r1844554589
########## pinot-common/src/main/java/org/apache/pinot/common/utils/request/BrokerRequestIdUtils.java: ########## @@ -0,0 +1,38 @@ +/** + * 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.common.utils.request; + +public class BrokerRequestIdUtils { + public static final int TABLE_TYPE_OFFSET = 10; Review Comment: Ya so this is definitely a little funkier than I'd like haha. This whole idea around borrowing the last digit of the request ID is _only_ for backward compatibility during deployment. The end goal for how multiple server queries per broker query would work is via requestId + physical table name being a unique identifier. That's implemented in this PR[1]. The challenge is that having the servers identify which physical table their response DataTable pertains to requires a change on the server code (to always include table name in the DataTable headers[2]). During upgrade, there will be a time where brokers are upgraded and expecting servers to return physical table name in DataTable headers, but servers won't yet be upgraded and won't be guaranteed to include that table name data. To bridge that period of time where brokers are updated but servers are not yet, my concept here is to use the request ID itself to inform brokers of the physical table name. With this first iteration of the Logical Table code in this PR, there is still only support for 1 broker request to hit 1 table: 1 realtime, 1 offline, or 1 hybrid. In the case of hybrid, 2 queries are sent to the server; this is where we use the request ID last digit to indicate whether the request was to a REALTIME or OFFLINE table. In the AsyncQueryResponse, we know the name of the table that the request was sent to based on the raw table name in the broker query, so we know any server response must pertain to that single raw table name. What we wouldn't know is REALTIME vs OFFLINE. So we set the last digit of the request ID _only_ when it goes to the server based on the physical table type. We can then map that request ID from the server back to the canonical request ID that the broker understands ( zero-out the last digit) in order to reduce the server responses properly in the broker. In a future release version, after servers are already guaranteed to be sending back table name in DataTable metadata, we will need to remove this entire concept of hijacking the request ID to determine offline vs realtime. I'd be happy to jump on a call/video call and walk through this more deeply as I admit it's fairly convoluted haha. [1] https://github.com/apache/pinot/pull/13742/files#diff-c7c33a86af22a88bd07f57bf2856504c7e6fa159e6cbc366b971e12051c90f23R243-R250 [2] https://github.com/apache/pinot/pull/13742/files#diff-2bff83abd3f6e831acfe4b6d31a022f228710def4eea47db3929c6d90b3147ecR157 -- 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
