morningman commented on code in PR #68902:
URL: https://github.com/apache/doris/pull/68902#discussion_r4244950130


##########
regression-test/suites/external_table_p0/kerberos/test_iceberg_hadoop_catalog_kerberos.groovy:
##########
@@ -97,6 +97,72 @@ suite("test_iceberg_hadoop_catalog_kerberos", "p0,external") 
{
           'partition_val2'
         );
     """
+    def snapshotsAfterFirstInsert = sql """
+        select snapshot_id from ${test_tbl_name}\$snapshots order by 
committed_at
+    """
+    assert snapshotsAfterFirstInsert.size() == 1
+    String firstSnapshotId = String.valueOf(snapshotsAfterFirstInsert[0][0])
+
+    sql """
+        insert into ${test_tbl_name} values (
+          '2024-05-27 12:34:56',
+          false,
+          456,
+          4567890123456,
+          45.67,
+          89.012,
+          45678.9012,
+          'another example',
+          '2024-05-27',
+          '2024-05-27 14:00:00',
+          'partition_val1',
+          'partition_val2'
+        );
+    """
+    def dataBeforeRollback = sql """select count(1) from ${test_tbl_name}"""
+    assert dataBeforeRollback.get(0).get(0) == 2
+
+    // Rollback must commit metadata with the same Kerberos identity used to 
acquire the table.
+    sql """
+        alter table ${catalog_name}.${database_name}.${test_tbl_name}
+        execute rollback_to_snapshot("snapshot_id" = "${firstSnapshotId}")
+    """
+    def dataAfterRollback = sql """select count(1) from ${test_tbl_name}"""
+    assert dataAfterRollback.get(0).get(0) == 1
+
+    sql """
+        insert into ${test_tbl_name} values (
+          '2024-05-27 12:34:56',
+          false,
+          456,
+          4567890123456,
+          45.67,
+          89.012,
+          45678.9012,
+          'another example',
+          '2024-05-27',
+          '2024-05-27 14:00:00',
+          'partition_val1',
+          'partition_val2'
+        );
+    """
+
+    def snapshotsBeforeExpire = sql """
+        select snapshot_id from ${test_tbl_name}\$snapshots order by 
committed_at
+    """
+    assert snapshotsBeforeExpire.size() == 3
+
+    // The action must reuse the catalog's Kerberos identity while committing 
new metadata.
+    sql """
+        alter table ${catalog_name}.${database_name}.${test_tbl_name}
+        execute expire_snapshots("retain_last" = "1")

Review Comment:
   Following up on the refresh part, which this round keeps open: it stays 
unchanged in this PR, for these reasons.
   
   - The boundary is not new. `ConnectorExecuteAction` refreshes only when a 
procedure returns, for every connector procedure, and branch-4.x's 
`ExecuteActionCommand` has the same boundary. This PR changes how the 
procedure's files are opened, not when the refresh runs.
   - The Kerberos failure from this thread no longer happens after the commit: 
the cleanup's worker-pool reads now run inside the plugin `doAs`.
   - For `expire_snapshots` the stale window is narrow. The commit drops old 
snapshots but does not move the current snapshot, so a cached table keeps 
reading the same current data; what can stay stale until the next refresh or 
cache expiry is the snapshot list and time travel to an expired snapshot.
   - Refreshing after a failed procedure is a change to the shared execute 
action: it would refresh after every failure, including those before any 
commit, or it needs a way to tell a failure after the commit from one before 
it. That belongs in its own change, separate from Kerberos file access.
   



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