Copilot commented on code in PR #59327:
URL: https://github.com/apache/doris/pull/59327#discussion_r2645068469
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -394,7 +395,7 @@ public TConfirmUnusedRemoteFilesResult
confirmUnusedRemoteFiles(TConfirmUnusedRe
return;
}
// check cooldownReplicaId
Review Comment:
The cast to LocalTablet is performed without any guard to verify the tablet
is actually a LocalTablet instance. This will cause a ClassCastException in
cloud mode where tablets are CloudTablet instances. Add a check for
Config.isNotCloudMode() or use instanceof LocalTablet before the cast.
```suggestion
// check cooldownReplicaId
if (!(tablet instanceof LocalTablet)) {
LOG.warn("tablet {} is not a LocalTablet, skip cooldown
confirm", info.tablet_id);
return;
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TabletInvertedIndex.java:
##########
@@ -672,14 +672,14 @@ private boolean needSync(Replica replicaInFe, TTabletInfo
backendTabletInfo) {
private void handleCooldownConf(TabletMeta tabletMeta, TTabletInfo
beTabletInfo,
List<CooldownConf> cooldownConfToPush, List<CooldownConf>
cooldownConfToUpdate) {
- Tablet tablet;
+ LocalTablet tablet;
try {
OlapTable table = (OlapTable)
Env.getCurrentInternalCatalog().getDbNullable(tabletMeta.getDbId())
.getTable(tabletMeta.getTableId())
.get();
table.readLock();
try {
- tablet =
table.getPartition(tabletMeta.getPartitionId()).getIndex(tabletMeta.getIndexId())
+ tablet = (LocalTablet)
table.getPartition(tabletMeta.getPartitionId()).getIndex(tabletMeta.getIndexId())
.getTablet(beTabletInfo.tablet_id);
Review Comment:
The cast to LocalTablet is unsafe and could throw ClassCastException in
cloud mode. The method handleCooldownConf should verify that the tablet is a
LocalTablet before casting, or add a guard to ensure this code only runs in
non-cloud mode. Consider adding an instanceof check or Config.isNotCloudMode()
guard.
```suggestion
Tablet baseTablet =
table.getPartition(tabletMeta.getPartitionId())
.getIndex(tabletMeta.getIndexId())
.getTablet(beTabletInfo.tablet_id);
if (!(baseTablet instanceof LocalTablet)) {
// In cloud mode or other configurations, the tablet may
not be a LocalTablet.
// This method only handles cooldown conf for
LocalTablet, so just return.
return;
}
tablet = (LocalTablet) baseTablet;
```
##########
fe/fe-core/src/main/java/org/apache/doris/master/ReportHandler.java:
##########
@@ -1533,7 +1534,7 @@ private static boolean addReplica(long tabletId,
TabletMeta tabletMeta, TTabletI
if (backendTabletInfo.isSetCooldownMetaId()) {
// replica has cooldowned data
do {
- Pair<Long, Long> cooldownConf =
tablet.getCooldownConf();
+ Pair<Long, Long> cooldownConf = ((LocalTablet)
tablet).getCooldownConf();
Review Comment:
The cast to LocalTablet is unsafe and could throw ClassCastException. The
code checks if cooldownMetaId is set, but this doesn't guarantee the tablet is
a LocalTablet. In cloud mode, tablets can be CloudTablet instances which don't
have the getCooldownConf method. Add a guard to check Config.isNotCloudMode()
or use instanceof LocalTablet before casting.
##########
fe/fe-core/src/main/java/org/apache/doris/cooldown/CooldownConfHandler.java:
##########
@@ -100,7 +101,7 @@ private void updateCooldownConf(List<CooldownConf>
confToUpdate) {
// update Tablet
for (CooldownConf conf : updatedConf) {
- Tablet tablet = tabletMap.get(conf.tabletId);
+ LocalTablet tablet = (LocalTablet) tabletMap.get(conf.tabletId);
Review Comment:
The cast to LocalTablet is performed without verifying the tablet type.
Since tabletMap.get() returns a Tablet which could be a CloudTablet instance,
this cast could fail with ClassCastException. Add an instanceof check before
casting.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -559,8 +560,8 @@ private void addScanRangeLocations(Partition partition,
}
}
- if (isEnableCooldownReplicaAffinity(context)) {
- final long coolDownReplicaId = tablet.getCooldownReplicaId();
+ if (Config.isNotCloudMode() &&
isEnableCooldownReplicaAffinity(context)) {
Review Comment:
The cast to LocalTablet is not safe because there's no instanceof check
before the cast. While Config.isNotCloudMode() guards the block, the tablet
variable could still be a CloudTablet in mixed environments or during
transitions. Add an instanceof check before casting to prevent
ClassCastException.
```suggestion
if (Config.isNotCloudMode() &&
isEnableCooldownReplicaAffinity(context) && tablet instanceof LocalTablet) {
```
##########
fe/fe-core/src/main/java/org/apache/doris/cooldown/CooldownConfHandler.java:
##########
@@ -83,7 +84,7 @@ private void updateCooldownConf(List<CooldownConf>
confToUpdate) {
int index = rand.nextInt(replicas.size());
conf.setCooldownReplicaId(replicas.get(index).getId());
// find TabletMeta to get cooldown term
- Tablet tablet = getTablet(conf);
+ LocalTablet tablet = (LocalTablet) getTablet(conf);
Review Comment:
The cast to LocalTablet lacks an instanceof check before casting. The
getTablet method returns a Tablet, which could be a CloudTablet in cloud mode.
Add an instanceof LocalTablet check before the cast to prevent
ClassCastException.
##########
fe/fe-core/src/main/java/org/apache/doris/cooldown/CooldownConfHandler.java:
##########
@@ -135,7 +136,7 @@ private static Tablet getTablet(CooldownConf conf) {
public static void replayUpdateCooldownConf(CooldownConfList
cooldownConfList) {
cooldownConfList.getCooldownConf().forEach(conf -> {
- Tablet tablet = getTablet(conf);
+ LocalTablet tablet = (LocalTablet) getTablet(conf);
if (tablet != null) {
tablet.setCooldownConf(conf.cooldownReplicaId,
conf.cooldownTerm);
Review Comment:
The cast to LocalTablet is done without an instanceof check. The getTablet
method returns a Tablet, which might not be a LocalTablet in all cases. Add an
instanceof check and handle the case where the tablet is not a LocalTablet to
prevent ClassCastException.
```suggestion
Tablet tablet = getTablet(conf);
if (tablet instanceof LocalTablet) {
((LocalTablet)
tablet).setCooldownConf(conf.cooldownReplicaId, conf.cooldownTerm);
} else if (tablet != null) {
LOG.warn("tablet is not LocalTablet when replaying cooldown
conf. tabletId={}", conf.tabletId);
```
--
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]
