kinolaev commented on PR #18258: URL: https://github.com/apache/iceberg/pull/18258#issuecomment-5981134615
Hi @Neuw84, thanks for the PR! I've run into a similar problem: building the equality delete set dominates the task. But in my case (CDC with https://github.com/memiiso/debezium-server-iceberg), there are many more data and delete files than the 64 and 616 in your benchmark, and the data files reference overlapping sets of delete files. As far as I understand, with this PR applied, a delete file that participates in several sets is loaded and cached once per set. For example, here are the ID ranges of the files from three commits: ``` deletes-01 1000-1999 data-01 1000-1999 deletes-02 1000-1799 data-02 1000-1799 deletes-03 1200-1999 data-03 1200-1999 ``` `data-01` references `deletes-02` and `deletes-03`, and `data-02` references `deletes-03`, so `deletes-03` is loaded and cached twice (under the `eq-delete-set|deletes-03` and `eq-delete-set|deletes-02|deletes-03` keys), while each cached set is used only once. Without this PR, `deletes-03` is loaded only once, and each merged set is discarded after it has been applied to its data file. So caching per location list works well when data files reference the same set of delete files, but it duplicates reads and memory for the shared files when two sets differ even by one file. Another concern is the key length. Even in your case, with 616 delete files, the key for the full set is probably larger than 64 KB, and keys are built and hashed for every split. This cost is negligible compared to rebuilding the delete set, but it can become significant when a set contains more delete files. I've been working on a related approach in #16956. I'd be happy if you could take a look and test it in your environment. It introduces an `org.apache.iceberg.data.EqualityDeletes` structure that can be shared by all equality deletes within a snapshot. For now, the PR uses it only to share equality deletes within a Spark task, but it might be a good idea to make it thread-safe and cache it per executor under a `namespace.table.snapshot` key, so that each Spark task only adds the equality deletes that haven't already been loaded by other tasks. It could be a great combination of the two approaches: your per-executor caching, plus sharing deletes between data files that reference different sets of delete files. --- If it's easier to test with prebuilt artifacts, I publish [iceberg-spark-runtime jars and container images](https://github.com/kinolaev/iceberg/packages/) built from Iceberg releases with #16956 and a few of my other fixes applied. The links below show exactly what's included on top of each release: - [1.11.0](https://github.com/apache/iceberg/compare/apache-iceberg-1.11.0...kinolaev:iceberg:apache-iceberg-1.11.0.5) - [1.12.0](https://github.com/apache/iceberg/compare/apache-iceberg-1.12.0...kinolaev:iceberg:apache-iceberg-1.12.0.1) -- 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]
