Copilot commented on code in PR #60467:
URL: https://github.com/apache/doris/pull/60467#discussion_r2761971099
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudPartition.java:
##########
@@ -255,6 +262,27 @@ public static List<Long> getSnapshotVisibleVersionFromMs(
return versions;
}
+ private static List<OlapTable> getTables(List<CloudPartition> partitions) {
+ Map<Long, OlapTable> tableMap = new HashMap<>();
+ for (CloudPartition partition : partitions) {
+ if (tableMap.containsKey(partition.getTableId())) {
+ continue;
+ }
+ Optional<Database> dbOptional =
Env.getCurrentInternalCatalog().getDb(partition.getId());
+ if (!dbOptional.isPresent()) {
+ continue;
+ }
+ Table table =
dbOptional.get().getTableNullable(partition.getTableId());
+ if (table == null) {
+ continue;
+ }
+ tableMap.put(partition.getTableId(), (OlapTable) table);
+ }
+ List<OlapTable> tables =
tableMap.values().stream().collect(Collectors.toList());
+ Collections.sort(tables, Comparator.comparingLong(o -> o.getId()));
+ return tables;
Review Comment:
The new helper `getTables` is using `partition.getId()` as the database ID
when calling `Env.getCurrentInternalCatalog().getDb(...)`. `partition.getId()`
is the partition ID, so this lookup will almost always fail and you’ll skip
acquiring version locks for the corresponding tables. This should instead use
the database identifier (e.g. `partition.getDbId()`) so that the correct
`Database` and `OlapTable` objects are found.
##########
gensrc/thrift/FrontendService.thrift:
##########
@@ -753,6 +753,23 @@ struct TFrontendPingFrontendRequest {
3: optional string deployMode
}
+struct TClodVersionInfo {
+ 1: optional i64 tableId
+ 2: optional i64 partitionId
+ 3: optional i64 version
+ 4: optional i64 versionUpdateTime
+}
Review Comment:
The struct name `TClodVersionInfo` appears to contain a typo (“Clod” instead
of “Cloud”), which will propagate into all generated client/server types and
APIs. Consider renaming it to `TCloudVersionInfo` (and updating all references)
before this Thrift API is consumed externally.
--
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]
