[fix](fe) Avoid redundant partition version fetch during SQL cache validation - #67125
Open
mymeiyi wants to merge 1 commit into
Open
[fix](fe) Avoid redundant partition version fetch during SQL cache validation#67125mymeiyi wants to merge 1 commit into
mymeiyi wants to merge 1 commit into
Conversation
…lidation ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: SQL cache validation fetched cloud partition versions in batch but discarded the result and used the FE partition map for the actual existence check. Remove the redundant fetch so validation no longer issues an unnecessary Meta Service RPC or dereferences missing partitions before the explicit existence check. ### Release note None ### Check List (For Author) - Test: No need to test (dead call removal); git diff --check passed - Behavior changed: Yes (SQL cache validation no longer refreshes unused partition versions) - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Removes a redundant cloud partition version fetch during SQL cache validation to avoid an unnecessary Meta Service RPC.
Changes:
- Removed the batch version fetch call (
getVersionInBatchForCloudMode) during table/partition validation. - Removed the associated
RpcExceptionhandling path for that call. - Cleaned up an unused
Collectionimport.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
488
to
493
| 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? |
| return IsChanged.CHANGED_AND_INVALIDATE_CACHE; | ||
| } | ||
|
|
||
| for (Long scanPartitionId : scanTable.getScanPartitions()) { |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review complete: I found no new non-duplicate blocking issue in this change.
Critical checkpoint conclusions:
- Goal and correctness: The deletion accomplishes the stated goal. At cache creation, Doris still records the OLAP table version and the partition-derived BE cache gate. At lookup, freshness is decided by the stored-versus-current table version plus explicit existence checks for every scanned partition. The removed cloud batch call refreshed partition-object caches, but its returned versions were never compared or passed to the FE/BE cache consumer. Non-cloud behavior is unchanged because that helper was a no-op there.
- Scope and reuse: The PR is a focused one-file deletion. The creation-time batch fetch remains in
CacheAnalyzer, where its partition versions and timestamps are actually consumed. Import cleanup is correct; the remainingRpcExceptionimport is still needed by the table-version check. - Concurrency and lifecycle: No shared state, ownership, or lock order is added. The lookup path already holds statement table read locks, so removing the possible Meta Service partition RPC shortens work under those locks. Cloud commit, peer-FE propagation, and periodic sync paths continue to maintain table-version freshness.
- Errors and observability: Table-version RPC failures still log and invalidate the SQL-cache entry, and the outer lookup boundary still invalidates on unexpected failures. The removed partition-RPC failure behavior is already covered by the existing Copilot thread, so I did not duplicate it.
- Configuration and compatibility: No configuration, persistence, protocol, storage-format, FE-BE variable, or rolling-upgrade contract changes are introduced. Cloud/non-cloud branches and the parallel creation-versus-validation paths remain consistent.
- Performance: The change removes sorting/list construction, version-cache locking, and a possible Meta Service batch RPC whose result was unused.
ScanTable.getScanPartitions()is now called once and is a direct Lombok field getter. - Tests: Existing SQL-cache regression coverage exercises invalidation after inserts, insert overwrite, add/drop/replace partition, and truncation, including cloud-specific behavior. This PR adds no dedicated no-RPC call-count test, and this review did not run builds or tests per the runner instructions. The live FE CheckStyle, title, license, formatter, secret, and large-file checks are green.
- User focus: No additional user-provided focus was specified.
Overall, the change is small, coherent, and safe based on the traced creation-to-lookup and cloud version-publication flows. Review status: complete, with zero new inline comments after duplicate fencing against both existing review threads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Summary: SQL cache validation fetched cloud partition versions but discarded the result, and then used the FE partition map for the actual existence check. So remove this version fetch to reduce an unnecessary Meta Service RPC.