[GitHub] [doris] eldenmoon commented on a diff in pull request #20732: [Bug](topn opt) Fix Two-Phase read when some rowset swept

["eldenmoon (via GitHub)" <gi...@apache.org>]

eldenmoon commented on code in PR #20732:
URL: https://github.com/apache/doris/pull/20732#discussion_r1228357872


##########
be/src/runtime/query_context.h:
##########
@@ -222,6 +228,14 @@ class QueryContext {
     taskgroup::TaskGroupPtr _task_group;
     std::unique_ptr<RuntimeFilterMgr> _runtime_filter_mgr;
     const TQueryOptions _query_options;
+
+    // When querying, certain structures need to be delayed in their release.
+    // Placing these structures in _reference_holder can extend their lifespan until the query is completed,
+    // and they are automatically released. For example, in two-phase delayed materialization queries,
+    // Rowsets need to be released after the query is finished to avoid premature release during the query process,
+    // which could lead to query exceptions.
+    std::vector<std::shared_ptr<void>> _reference_holder;

Review Comment:
   done



##########
be/src/service/internal_service.cpp:
##########
@@ -1512,11 +1512,18 @@ Status PInternalServiceImpl::_multi_get(const PMultiGetRequest& request,
         if (!tablet) {
             continue;
         }
+        // Get Rowset from either tablet or unused rowsets, since this rowset maybe expired and swept.
+        // But we ensured it's rowset is not released when init Tablet reader param, by QueryContext::hold_reference
         BetaRowsetSharedPtr rowset =
                 std::static_pointer_cast<BetaRowset>(tablet->get_rowset(rowset_id));
         if (!rowset) {
-            LOG(INFO) << "no such rowset " << rowset_id;
-            continue;
+            std::optional<RowsetSharedPtr> from_unused_rowset =

Review Comment:
   done



##########
be/src/olap/storage_engine.cpp:
##########
@@ -1155,10 +1155,15 @@ Status StorageEngine::execute_task(EngineTask* task) {
 }
 
 // check whether any unused rowsets's id equal to rowset_id
-bool StorageEngine::check_rowset_id_in_unused_rowsets(const RowsetId& rowset_id) {
+std::optional<RowsetSharedPtr> StorageEngine::get_rowset_in_unused_rowsets(

Review Comment:
   done



-- 
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