yuqi1129 commented on code in PR #13487:
URL: https://github.com/apache/gravitino/pull/13487#discussion_r4230060057
##########
api/src/main/java/org/apache/gravitino/stats/SupportsStatistics.java:
##########
@@ -53,9 +61,16 @@ void updateStatistics(Map<String, StatisticValue<?>>
statistics)
* Drop statistics by their names. If the statistic is unmodifiable, it will
throw an
* UnmodifiableStatisticException.
*
+ * <p>For Gravitino-managed table statistics, a concurrent modification or
same-name replacement
+ * can fail the entire drop with {@link OptimisticLockException}. No part of
that drop is
+ * committed; retry the operation against the current state. A statistic
already removed by
+ * another drop is not a conflict and is not counted as dropped by this call.
Review Comment:
Fixed in 7d62fe9c53: the dropStatistics Javadoc now matches updateStatistics
and explicitly leaves abort/retry decisions to the caller. Both
table-statistics OpenAPI 409 descriptions use the same caller-controlled
wording. spotlessApply, API Javadoc and :docs:build passed.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/StatisticMetaService.java:
##########
@@ -63,10 +77,23 @@ public List<StatisticEntity> listStatisticsByEntity(
.collect(Collectors.toList());
}
+ /**
+ * Creates or replaces statistics of a metadata object by name.
+ *
+ * <p>Existing statistics are replaced only if their version is unchanged
since this call read
+ * them, and missing statistics are inserted only if nobody created them
meanwhile; both cases
+ * otherwise fail the whole batch with {@link OptimisticLockException}. The
target is fenced in
+ * the same transaction, so a target dropped or replaced after its ID was
resolved fails with
+ * {@link NoSuchEntityException}.
+ *
+ * @param statisticEntities the statistics to write; names must be unique in
the batch
+ * @param entity the metadata object that owns the statistics
+ * @param type the metadata object type
+ */
@Monitored(
metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "batchInsertStatisticPOsOnDuplicateKeyUpdate")
- public void batchInsertStatisticPOsOnDuplicateKeyUpdate(
+ public void writeStatisticsWithVersion(
Review Comment:
Fixed in 7d62fe9c53: added a comment above @Monitored explaining that the
historical metric name is preserved for compatibility with existing monitoring.
The baseMetricName literal remains unchanged.
--
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]
