Copilot commented on code in PR #60072:
URL: https://github.com/apache/doris/pull/60072#discussion_r2707791405
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -2250,7 +2250,9 @@ public Map<Tag, List<List<Long>>>
getArbitraryTabletBucketsSeq() throws DdlExcep
public long proximateRowCount() {
long totalCount = 0;
for (Partition partition : getPartitions()) {
- long version = partition.getVisibleVersion();
+ // for local mode, getCachedVisibleVersion return visibleVersion.
+ // for cloud mode, the replica.checkVersionCatchUp always returns
true.
Review Comment:
The comment states "the replica.checkVersionCatchUp always returns true" for
cloud mode, but this is inaccurate. Looking at
CloudReplica.checkVersionCatchUp, it simply returns true unconditionally. The
comment should be rephrased to say "for cloud mode, checkVersionCatchUp always
returns true" or "for cloud mode, replica version checking is simplified" to
avoid implying that this is a property of the method call itself.
```suggestion
// for local mode, getCachedVisibleVersion returns
visibleVersion.
// for cloud mode, checkVersionCatchUp always returns true.
```
##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/RowCountAction.java:
##########
@@ -75,7 +75,9 @@ protected Object rowcount(HttpServletRequest request,
HttpServletResponse respon
olapTable.readLock();
try {
for (Partition partition : olapTable.getAllPartitions()) {
- long version = partition.getVisibleVersion();
+ // for local mode, getCachedVisibleVersion return
visibleVersion.
+ // for cloud mode, the replica.checkVersionCatchUp always
returns true.
Review Comment:
The comment states "the replica.checkVersionCatchUp always returns true" for
cloud mode, but this is inaccurate. Looking at
CloudReplica.checkVersionCatchUp, it simply returns true unconditionally. The
comment should be rephrased to say "for cloud mode, checkVersionCatchUp always
returns true" or "for cloud mode, replica version checking is simplified" to
avoid implying that this is a property of the method call itself.
```suggestion
// for cloud mode, checkVersionCatchUp always returns true
(replica version checking is simplified).
```
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java:
##########
@@ -193,7 +193,10 @@ protected Pair<List<Long>, Long> getSampleTablets() {
for (int i = 0; i < tabletCounts; i++) {
int seekTid = (int) ((i + seek) % ids.size());
long tabletId = ids.get(seekTid);
- long tabletRows =
materializedIndex.getTablet(tabletId).getMinReplicaRowCount(p.getVisibleVersion());
+ // for local mode, getCachedVisibleVersion return
visibleVersion.
+ // for cloud mode, the replica.checkVersionCatchUp always
returns true.
Review Comment:
The comment states "the replica.checkVersionCatchUp always returns true" for
cloud mode, but this is inaccurate. Looking at
CloudReplica.checkVersionCatchUp, it simply returns true unconditionally (after
the removal of the conditional checks). The comment should be rephrased to say
"for cloud mode, checkVersionCatchUp always returns true" or "for cloud mode,
replica version checking is simplified" to avoid implying that this is a
property of the method call itself.
```suggestion
// for cloud mode, checkVersionCatchUp always returns true.
```
--
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]
