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


##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsAction.java:
##########
@@ -221,14 +229,14 @@ protected List<String> executeAction(Table icebergTable, 
ConnectorSession sessio
 
             // Set up delete callback to count files by type
             expireSnapshots.deleteWith(path -> {
-                FileContent deleteContent = deleteFileContentByPath.get(path);
-                if (deleteContent == FileContent.POSITION_DELETES) {
+                FileKind fileKind = fileKindByPath.get(path);
+                if (fileKind == FileKind.POSITION_DELETES) {
                     deletedPositionDeleteFilesCount.incrementAndGet();
-                } else if (deleteContent == FileContent.EQUALITY_DELETES) {
+                } else if (fileKind == FileKind.EQUALITY_DELETES) {
                     deletedEqualityDeleteFilesCount.incrementAndGet();
-                } else if (path.contains("-m-") && path.endsWith(".avro")) {
+                } else if (fileKind == FileKind.MANIFEST) {
                     deletedManifestFilesCount.incrementAndGet();
-                } else if (path.contains("snap-") && path.endsWith(".avro")) {
+                } else if (fileKind == FileKind.MANIFEST_LIST) {

Review Comment:
   [P2] Account for snapshots added while the kind map is built
   
   `fileKindByPath` inventories the loaded snapshots, but Iceberg 1.11.0's 
`RemoveSnapshots.internalApply()` refreshes metadata at commit. If another 
writer appends S1 and S2 while this map is scanned, both before the 
`retain_last=1` cutoff is chosen, expiration keeps S2 and deletes S1's manifest 
list. That `snap-*.avro` path is absent from the map and now falls through to 
`deletedDataFilesCount`; the previous filename branch counted it correctly as a 
manifest list. An append-only table can therefore return `[1,0,0,0,1,0]` 
instead of `[0,0,0,0,2,0]`. Cover the refreshed expiration inventory, or 
preserve a correct fallback for unmapped metadata paths, and add a 
deterministic refresh-interleaving test.



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