github-actions[bot] commented on code in PR #68196:
URL: https://github.com/apache/doris/pull/68196#discussion_r4093127773
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -730,11 +730,17 @@ private void refreshMetaCacheOnly(boolean invalidCache) {
// prevents one full SDK-cache scan per cached database without
affecting concurrent
// expiry callbacks on other threads.
invalidateEngineCacheOnDatabaseRemoval.set(!invalidCache);
+ invalidatingAllMetaCache = true;
Review Comment:
[P1] Fence row counts before exposing the refreshed metadata generation.
metaCache.invalidateAll() installs a new live object-cache generation before
its retired callbacks finish, but both invalidCache modes publish their first
catalog row-count fence only afterward. A concurrent reader can rebuild the
same deterministic DB/table IDs in that interval and consume the retired
generation's completed RowCountKey future; the later fence cannot retract the
value already used for planning. Publish an opening catalog fence before the
generation swap (and retain the completion fence for loads admitted during
refresh), with a latch test between retireObjects() and the closing fence.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetadataOps.java:
##########
@@ -323,6 +323,10 @@ public void afterDropTable(String dbName, String tblName) {
Optional<ExternalDatabase<?>> db = catalog.getDbForReplay(dbName);
if (db.isPresent()) {
db.get().unregisterTable(tblName);
+ } else {
Review Comment:
[P1] Distinguish a cold DB object from a lost canonical mapping before
retiring the catalog. getDbForReplay() is cache-only, so it also returns empty
when the mapping is still known but this one DB object was evicted; this new
branch then retires every resident DB plus every engine and row-count entry for
a single DROP TABLE replay. That turns normal bounded-cache churn into
catalog-wide reloads (the same branch exists in Iceberg and MaxCompute).
Expose/check the replay-safe canonical mapping and use DB-scoped invalidation
when it is known; reserve this fallback for a genuinely absent mapping, with an
unrelated-hot-DB regression.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/HiveInsertExecutor.java:
##########
@@ -83,17 +83,40 @@ protected void doBeforeCommit() throws UserException {
protected void doAfterCommit() throws DdlException {
HMSExternalTable hmsTable = (HMSExternalTable) table;
+ // The transaction is already committed. Fence the row-count cache by
the held table
+ // identity before any fallible cache work (including
isPartitionedTable reinitialization),
+ // so an evicted or partially reloaded table cannot retain the
pre-insert count.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(hmsTable);
+
// For partitioned tables, do selective partition refresh
// For non-partitioned tables, do full table cache invalidation
List<String> modifiedPartNames = Lists.newArrayList();
List<String> newPartNames = Lists.newArrayList();
- if (hmsTable.isPartitionedTable() && partitionUpdates != null &&
!partitionUpdates.isEmpty()) {
- HiveExternalMetaCache cache =
Env.getCurrentEnv().getExtMetaCacheMgr()
- .hive(hmsTable.getCatalog().getId());
- cache.refreshAffectedPartitions(hmsTable, partitionUpdates,
modifiedPartNames, newPartNames);
- } else {
- // Non-partitioned table or no partition updates, do full table
refresh
-
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateTableCache(hmsTable);
+ try {
+ if (hmsTable.isPartitionedTable() && partitionUpdates != null &&
!partitionUpdates.isEmpty()) {
+ HiveExternalMetaCache cache =
Env.getCurrentEnv().getExtMetaCacheMgr()
+ .hive(hmsTable.getCatalog().getId());
+ cache.refreshAffectedPartitions(hmsTable, partitionUpdates,
modifiedPartNames, newPartNames);
+ // Close the admission window opened by the fence above: a
load admitted after it can
+ // compute the pre-insert value from the still-resident file
list and publish it.
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateRowCountCache(hmsTable);
Review Comment:
[P1] Refresh the held table-statistics source before closing this fence. The
selective path leaves HMSExternalTable.objectCreated/remoteTable intact, and a
load starting after this call first returns any positive pre-commit numRows or
totalSize from that object before it consults the refreshed file caches.
HMSTransaction updates separately fetched table/partition objects, so it does
not refresh this instance; follower/event selective paths and the leader full
invalidateTableCache branch have the same source-retention gap. Retire or
refresh the held table metadata (or bypass those stale parameters after commit)
and add a real-cache reload regression.
--
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]
