roryqi commented on PR #13532: URL: https://github.com/apache/gravitino/pull/13532#issuecomment-6121123355
Could we first consider a single-writer design for this use case instead of adding a distributed merge API? The dependent collector in #13717 updates one `spec_id` per job, but it already iterates `snapshot.allManifests(table.io())` and then filters to that spec. If several specs are collected independently, we may scan the complete manifest list multiple times and issue multiple Gravitino writes for the same table. An alternative would be: 1. Pin one snapshot per table collection cycle. 2. Traverse its manifests once and group the two measurements by `partitionSpecId`. 3. Have one logical writer per table publish the two complete object-valued statistics in one existing PUT/update call. 4. Enforce per-table single-flight in the optimizer/job layer, and serialize cycles or carry the snapshot ID so an older job cannot finish later and overwrite newer measurements. This would reduce both manifest scanning and Gravitino lock/database pressure, and it keeps ownership clear: the manifest collector is the sole writer of these two statistic names. With #13487 now providing atomic batch writes and OCC for the existing statistics update path, PUT already has the persistence guarantees needed by such a design. The current need for merge appears to come mainly from choosing one independently writable job per spec and explicitly avoiding scheduler/framework coordination. That is a design choice rather than an inherent requirement of the statistics. Before adding a general PATCH/`mergeStatistics` surface, could we document why per-table single-writer collection is insufficient or too costly? There is also a correctness concern after #13487: this PR computes the merged value in `StatisticManager` before the storage layer takes its version snapshot. On another Gravitino instance, a concurrent write can commit between those two reads; the storage layer may then accept the current version while writing a merge result computed from stale values, silently losing the other update. If multi-writer merge remains the chosen design, the read/merge/version-check/write operation should be moved into one storage transaction or equivalent CAS retry boundary. -- 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]
