github-actions[bot] commented on code in PR #65282:
URL: https://github.com/apache/doris/pull/65282#discussion_r3548498681
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -5228,6 +5210,94 @@ public TReplacePartitionResult
replacePartition(TReplacePartitionRequest request
return result;
}
+ private static final class PartitionResultSnapshot {
+ private final Partition partition;
+ private final long partitionId;
+ private final TOlapTablePartition tPartition;
+ private final List<Tablet> tablets;
+ private final int quorum;
+ private final boolean cacheLoadTabletIdx;
+
+ private PartitionResultSnapshot(Partition partition, long partitionId,
+ TOlapTablePartition tPartition, List<Tablet> tablets, int
quorum, boolean cacheLoadTabletIdx) {
+ this.partition = partition;
+ this.partitionId = partitionId;
+ this.tPartition = tPartition;
+ this.tablets = tablets;
+ this.quorum = quorum;
+ this.cacheLoadTabletIdx = cacheLoadTabletIdx;
+ }
Review Comment:
This new retry path should have automated coverage before we rely on it. The
PR now converts a concurrently missing result partition or missing
`PartitionItem` into a `UserException` that the callers return as a non-OK
status, but the live PR changes only `FrontendServiceImpl.java`, so there is no
FE unit/regression test proving either
`snapshotPartitionResultsByName`/`snapshotPartitionResultsById` returns the
retry status instead of throwing the old NPE. Please add a focused FE test or
debug-point regression that forces this helper to see a missing partition/item
and asserts the RPC returns the new failure status.
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -5228,6 +5210,94 @@ public TReplacePartitionResult
replacePartition(TReplacePartitionRequest request
return result;
}
+ private static final class PartitionResultSnapshot {
+ private final Partition partition;
+ private final long partitionId;
+ private final TOlapTablePartition tPartition;
+ private final List<Tablet> tablets;
+ private final int quorum;
+ private final boolean cacheLoadTabletIdx;
+
+ private PartitionResultSnapshot(Partition partition, long partitionId,
+ TOlapTablePartition tPartition, List<Tablet> tablets, int
quorum, boolean cacheLoadTabletIdx) {
+ this.partition = partition;
+ this.partitionId = partitionId;
+ this.tPartition = tPartition;
+ this.tablets = tablets;
+ this.quorum = quorum;
+ this.cacheLoadTabletIdx = cacheLoadTabletIdx;
+ }
+ }
+
+ private static List<PartitionResultSnapshot>
snapshotPartitionResultsByName(OlapTable olapTable,
+ Collection<String> partitionNames, boolean loadToSingleTablet,
boolean enableAdaptiveRandomBucket,
+ String resultName) throws UserException {
+ PartitionInfo partitionInfo = olapTable.getPartitionInfo();
+ int partColNum = partitionInfo.getPartitionColumns().size();
+ List<PartitionResultSnapshot> partitionSnapshots = new ArrayList<>();
+ for (String partitionName : partitionNames) {
+ Partition partition = olapTable.getPartition(partitionName);
+ if (partition == null) {
Review Comment:
This new retry path should have automated coverage before we rely on it. The
PR now converts a concurrently missing result partition or missing
`PartitionItem` into a `UserException` that the callers return as a non-OK
status, but the live PR changes only `FrontendServiceImpl.java`, so there is no
FE unit/regression test proving either
`snapshotPartitionResultsByName`/`snapshotPartitionResultsById` returns the
retry status instead of throwing the old NPE. Please add a focused FE test or
debug-point regression that forces this helper to see a missing partition/item
and asserts the RPC returns the new failure status.
--
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]
