You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "mymeiyi (via GitHub)" <gi...@apache.org> on 2026/04/15 06:10:28 UTC

[PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

mymeiyi opened a new pull request, #62514:
URL: https://github.com/apache/doris/pull/62514

   pick https://github.com/apache/doris/pull/60705


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-4411710512

   run buildall


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-4250042808

   # FE UT Coverage Report
   Increment line coverage `  6.59% (6/91)` :tada:
   [Increment coverage report](http://coverage.selectdb-in.cc/coverage/62514_6f816ed8819a4723976d72fea3a1489f297a2a23/fe_increment_report/index.html)
   [Complete coverage report](http://coverage.selectdb-in.cc/coverage/62514_6f816ed8819a4723976d72fea3a1489f297a2a23/fe_report/index.html)
   


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-4249723557

   
   Thank you for your contribution to Apache Doris.
   Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
   
   Please clearly describe your PR:
   1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
   2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
   3. What features were added. Why was this function added?
   4. Which code was refactored and why was this part of the code refactored?
   5. Which functions were optimized and what is the difference before and after the optimization?
       


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-4249727088

   run buildall


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-5042994850

   # FE UT Coverage Report
   Increment line coverage `  6.59% (6/91)` :tada:
   [Increment coverage report](http://coverage.selectdb-in.cc/coverage/62514_4b559ef5cb4ff299d0ea6dfe4ba5f4410e5a8c2a/fe_increment_report/index.html)
   [Complete coverage report](http://coverage.selectdb-in.cc/coverage/62514_4b559ef5cb4ff299d0ea6dfe4ba5f4410e5a8c2a/fe_report/index.html)
   


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi closed pull request #62514: branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705)
URL: https://github.com/apache/doris/pull/62514


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #62514:
URL: https://github.com/apache/doris/pull/62514#discussion_r3084366002


##########
fe/fe-core/src/main/java/org/apache/doris/master/Checkpoint.java:
##########
@@ -395,4 +417,100 @@ private long getMemoryUsedPercent() {
     public ReentrantReadWriteLock getLock() {
         return lock;
     }
+
+    private void postProcessCloudMetadata() {
+        if (Config.isNotCloudMode()) {
+            return;
+        }
+        Env servingEnv = Env.getServingEnv();
+        if (servingEnv == null) {
+            LOG.warn("serving env is null, skip process cloud metadata for checkpoint");
+            return;
+        }
+        long start = System.currentTimeMillis();
+        for (Database db : env.getInternalCatalog().getDbs()) {
+            Database servingDb = servingEnv.getInternalCatalog().getDbNullable(db.getId());
+            if (servingDb == null) {
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("serving db is null. dbId: {}, dbName: {}", db.getId(), db.getFullName());
+                }
+                continue;
+            }
+
+            for (Table table : db.getTables()) {
+                Table servingTable = servingDb.getTableNullable(table.getId());
+                if (servingTable == null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("serving table is null. dbId: {}, table: {}", db.getId(), table);
+                    }
+                    continue;
+                }
+                if (!(table instanceof OlapTable) || !(servingTable instanceof OlapTable)) {
+                    continue;
+                }
+                OlapTable olapTable = (OlapTable) table;
+                OlapTable servingOlapTable = (OlapTable) servingTable;
+
+                List<Partition> partitions = olapTable.getAllPartitions();
+                for (Partition partition : partitions) {
+                    Partition servingPartition = servingOlapTable.getPartition(partition.getId());
+                    if (servingPartition == null) {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("serving partition is null. tableId: {}, partitionId: {}", table.getId(),
+                                    partition.getId());
+                        }
+                        continue;
+                    }
+                    // set tablet stats
+                    setTabletStats(table.getId(), partition, servingPartition);
+                }
+            }
+        }
+        LOG.info("post process cloud metadata for checkpoint finished. cost {} ms", System.currentTimeMillis() - start);
+    }
+
+    private void setTabletStats(long tableId, Partition partition, Partition servingPartition) {
+        for (MaterializedIndex index : partition.getMaterializedIndices(IndexExtState.VISIBLE)) {
+            MaterializedIndex servingIndex = servingPartition.getIndex(index.getId());
+            if (servingIndex == null) {
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("serving index is null. tableId: {}, partitionId: {}, indexId: {}", tableId,
+                            partition.getId(), index.getId());
+                }
+                continue;
+            }
+            for (Tablet tablet : index.getTablets()) {
+                Tablet servingTablet = servingIndex.getTablet(tablet.getId());
+                if (servingTablet == null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("serving tablet is null. tableId: {}, partitionId: {}, indexId: {}, tabletId: {}",
+                                tableId, partition.getId(), index.getId(), tablet.getId());
+                    }
+                    continue;
+                }
+                for (Replica replica : tablet.getReplicas()) {
+                    Replica servingReplica = servingTablet.getReplicaById(replica.getId());
+                    if (servingReplica == null) {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("serving replica is null. tableId: {}, partitionId: {}, indexId: {}, "
+                                            + "tabletId: {}, replicaId: {}", tableId, partition.getId(), index.getId(),
+                                    tablet.getId(), replica.getId());
+                        }
+                        continue;
+                    }
+                    replica.setDataSize(servingReplica.getDataSize());
+                    replica.setRowsetCount(servingReplica.getRowsetCount());
+                    replica.setSegmentCount(servingReplica.getSegmentCount());
+                    replica.setRowCount(servingReplica.getRowCount());
+                    replica.setLocalInvertedIndexSize(servingReplica.getLocalInvertedIndexSize());
+                    replica.setLocalSegmentSize(servingReplica.getLocalSegmentSize());
+                    // set last get stats time and stats interval index
+                    CloudReplica cloudReplica = (CloudReplica) replica;
+                    CloudReplica servingCloudReplica = (CloudReplica) servingReplica;
+                    cloudReplica.setStatsIntervalIndex(servingCloudReplica.getStatsIntervalIndex());
+                    cloudReplica.setLastGetTabletStatsTime(servingCloudReplica.getLastGetTabletStatsTime());

Review Comment:
   `setTabletStats()` unconditionally casts both `replica` and `servingReplica` to `CloudReplica`. In cloud mode, the image loader can still deserialize replicas as base `Replica` (e.g., compatibility subtype `LocalReplica` or default subtype), so this can throw `ClassCastException` and fail the whole checkpoint. Please guard with `instanceof` (and only copy the cloud-specific fields when both sides are `CloudReplica`), so checkpoint remains best-effort for stats copying.
   ```suggestion
                       if (replica instanceof CloudReplica && servingReplica instanceof CloudReplica) {
                           CloudReplica cloudReplica = (CloudReplica) replica;
                           CloudReplica servingCloudReplica = (CloudReplica) servingReplica;
                           cloudReplica.setStatsIntervalIndex(servingCloudReplica.getStatsIntervalIndex());
                           cloudReplica.setLastGetTabletStatsTime(servingCloudReplica.getLastGetTabletStatsTime());
                       }
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/master/Checkpoint.java:
##########
@@ -395,4 +417,100 @@ private long getMemoryUsedPercent() {
     public ReentrantReadWriteLock getLock() {
         return lock;
     }
+
+    private void postProcessCloudMetadata() {
+        if (Config.isNotCloudMode()) {
+            return;
+        }
+        Env servingEnv = Env.getServingEnv();
+        if (servingEnv == null) {
+            LOG.warn("serving env is null, skip process cloud metadata for checkpoint");
+            return;
+        }
+        long start = System.currentTimeMillis();
+        for (Database db : env.getInternalCatalog().getDbs()) {
+            Database servingDb = servingEnv.getInternalCatalog().getDbNullable(db.getId());
+            if (servingDb == null) {
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("serving db is null. dbId: {}, dbName: {}", db.getId(), db.getFullName());
+                }
+                continue;
+            }
+
+            for (Table table : db.getTables()) {
+                Table servingTable = servingDb.getTableNullable(table.getId());
+                if (servingTable == null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("serving table is null. dbId: {}, table: {}", db.getId(), table);
+                    }
+                    continue;
+                }
+                if (!(table instanceof OlapTable) || !(servingTable instanceof OlapTable)) {
+                    continue;
+                }
+                OlapTable olapTable = (OlapTable) table;
+                OlapTable servingOlapTable = (OlapTable) servingTable;
+
+                List<Partition> partitions = olapTable.getAllPartitions();
+                for (Partition partition : partitions) {
+                    Partition servingPartition = servingOlapTable.getPartition(partition.getId());
+                    if (servingPartition == null) {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("serving partition is null. tableId: {}, partitionId: {}", table.getId(),
+                                    partition.getId());
+                        }
+                        continue;
+                    }
+                    // set tablet stats
+                    setTabletStats(table.getId(), partition, servingPartition);

Review Comment:
   `postProcessCloudMetadata()` traverses `servingEnv` metadata (tables/partitions/indexes/tablets) without taking the usual read locks. `Partition`’s index maps are plain HashMaps, so concurrent DDL/rollup updates on the serving catalog can lead to inconsistent reads or `ConcurrentModificationException` during checkpoint. Consider acquiring `servingOlapTable.readLock()` (and releasing in `finally`) while you resolve partitions/index/tablet/replica lookups (or otherwise snapshot the needed structures under lock).
   ```suggestion
                   servingOlapTable.readLock();
                   try {
                       for (Partition partition : partitions) {
                           Partition servingPartition = servingOlapTable.getPartition(partition.getId());
                           if (servingPartition == null) {
                               if (LOG.isDebugEnabled()) {
                                   LOG.debug("serving partition is null. tableId: {}, partitionId: {}", table.getId(),
                                           partition.getId());
                               }
                               continue;
                           }
                           // set tablet stats
                           setTabletStats(table.getId(), partition, servingPartition);
                       }
                   } finally {
                       servingOlapTable.readUnlock();
   ```



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "mymeiyi (via GitHub)" <gi...@apache.org>.
mymeiyi commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-4395115635

   run buildall


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] branch-4.0: [fix](cloud) checkpoint save cloud tablet stats to image (#60705) [doris]

Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on PR #62514:
URL: https://github.com/apache/doris/pull/62514#issuecomment-5042528925

   run buildall


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org