morningman commented on code in PR #69043:
URL: https://github.com/apache/doris/pull/69043#discussion_r4272395263
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsAction.java:
##########
@@ -311,7 +337,31 @@ Map<String, FileContent> buildDeleteFileContentMap(Table
icebergTable) {
throw new DorisConnectorException("Failed to build delete file
content map: " + e.getMessage(), e);
}
lastDeleteManifestReadCount = reads;
- return deleteFileContentByPath;
+ return fileKindByPath;
+ }
+
+ /**
+ * Tells a manifest or manifest list that {@link #buildFileKindMap} did
not see apart by its file name. The
+ * expiration refreshes the table when it commits, so it also expires
snapshots that another writer committed
+ * after the map was built. SnapshotProducer names a manifest list
+ * {@code snap-<snapshot id>-<attempt>-<commit UUID>.avro} and a manifest
{@code <commit UUID>-m<N>.avro};
+ * Spark's {@code rewrite_manifests} names a manifest {@code
optimized-m-<UUID>.avro}. Returns null for any
+ * other file. A delete file of such a snapshot still counts as a data
file, as its name does not tell the
+ * two apart.
+ */
+ @VisibleForTesting
+ static FileKind metadataFileKindByName(String path) {
+ String fileName = path.substring(path.lastIndexOf('/') + 1);
+ if (!fileName.endsWith(".avro")) {
+ return null;
+ }
+ if (fileName.startsWith("snap-")) {
+ return FileKind.MANIFEST_LIST;
+ }
+ if (fileName.contains("-m-") ||
MANIFEST_FILE_NAME.matcher(fileName).matches()) {
Review Comment:
Fixed in aeccc7c414a. You are right: the map holds no data files, so every
data file takes the name fallback, and `-m<digits>.avro` also matched an Avro
data file such as `sales-m1.avro`.
A name now counts only when the whole file name matches the name Iceberg
gives the file, UUID included: `snap-<snapshot id>-<attempt>-<commit
UUID>.avro` (`SnapshotProducer.manifestListPath`) is a manifest list, and
`<commit UUID>-m<N>.avro` (`SnapshotProducer.newManifestOutputFile`) or
`optimized-m-<UUID>.avro` (Spark's `RewriteManifestsSparkAction`) is a
manifest. Names such as `snap-1.avro` or `sales-m-1.avro`, which the earlier
`snap-` prefix and `-m-` checks matched, no longer count as metadata either.
Tests:
- `countsAnAvroDataFileNamedLikeAManifestAsADataFile` is your case: it
appends `s3://b/db1/t/data/sales-m1.avro`, deletes it, appends `f2.parquet`,
and expires with `older_than=4102444800000, retain_last=1`. It expects `[1, 0,
0, 2, 2, 0]`; with the previous rule it gets `[0, 0, 0, 3, 2, 0]`.
- `classifiesUnmappedMetadataFilesByIcebergFileNames` also checks that
`sales-m1.avro`, `sales-m-1.avro`, `optimized-m-1.avro`, `snap-1.avro` and
`snap-1234-1-sales.avro` in a data directory are not metadata.
--
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]
