Copilot commented on code in PR #67125:
URL: https://github.com/apache/doris/pull/67125#discussion_r3851956206
##########
fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java:
##########
@@ -489,14 +488,6 @@ private IsChanged tablesOrDataChanged(Env env,
SqlCacheContext sqlCacheContext)
TableIf tableIf = findTableIf(env, fullTableName);
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
- Collection<Long> partitionIds = scanTable.getScanPartitions();
- try {
- olapTable.getVersionInBatchForCloudMode(partitionIds);
- } catch (RpcException e) {
- LOG.warn("failed to get version in batch for table {}",
fullTableName, e);
- return IsChanged.CHANGED_AND_INVALIDATE_CACHE;
- }
-
for (Long scanPartitionId : scanTable.getScanPartitions()) {
Review Comment:
`scanTable.getScanPartitions()` is invoked multiple times in this method (it
was also used just above in the removed block). If `getScanPartitions()` does
any non-trivial computation/allocation, store it in a local variable once and
reuse it to avoid repeated work.
##########
fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java:
##########
@@ -489,14 +488,6 @@ private IsChanged tablesOrDataChanged(Env env,
SqlCacheContext sqlCacheContext)
TableIf tableIf = findTableIf(env, fullTableName);
if (tableIf instanceof OlapTable) {
OlapTable olapTable = (OlapTable) tableIf;
- Collection<Long> partitionIds = scanTable.getScanPartitions();
- try {
- olapTable.getVersionInBatchForCloudMode(partitionIds);
- } catch (RpcException e) {
- LOG.warn("failed to get version in batch for table {}",
fullTableName, e);
- return IsChanged.CHANGED_AND_INVALIDATE_CACHE;
- }
-
for (Long scanPartitionId : scanTable.getScanPartitions()) {
Partition partition =
olapTable.getPartition(scanPartitionId);
// partition == null: is this partition truncated?
Review Comment:
Removing the Meta Service RPC also removes the previous failure-driven
invalidation behavior (previously any `RpcException` would invalidate the
cache). If the intent is that FE metadata alone is sufficient/authoritative for
validation, consider adding an explicit comment here to document that
assumption; otherwise, consider retaining a lightweight health/error signal so
Meta Service outage doesn’t silently allow cache validation to pass on
potentially stale FE state.
--
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]
