github-actions[bot] commented on code in PR #69950:
URL: https://github.com/apache/doris/pull/69950#discussion_r4451027856


##########
be/src/service/http/action/check_encryption_action.cpp:
##########
@@ -64,7 +65,12 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr& 
tablet) {
     }
     Status st;
     bool is_encrypted = true;
-    tablet->traverse_rowsets([&st, &tablet, &is_encrypted](const 
RowsetSharedPtr& rs) {
+    // The files are read without holding the tablet's meta lock: `_meta_lock` 
is writer-preferring,
+    // so taking it again for the schema while a writer is pending would 
deadlock, and holding it
+    // across remote reads would stall every other user of this tablet.

Review Comment:
   [P1] Keep the schema and rowset list from one `_meta_lock` snapshot. 
`tablet_schema()` releases `_meta_lock` before `get_snapshot_rowset()` acquires 
it, so a concurrent index-change compaction can publish a new rowset between 
those calls while this function still uses the old schema. The new rowset's V2 
index file is then omitted from `file_paths`, allowing an unencrypted index to 
be reported as fully encrypted (and the opposite ordering can spuriously fail 
opening a V2 path on an old rowset). Capture both values under one shared lock, 
then release it before the remote file reads.



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

Reply via email to