yuqi1129 commented on code in PR #13737:
URL: https://github.com/apache/gravitino/pull/13737#discussion_r4245792145
##########
core/src/main/java/org/apache/gravitino/stats/StatisticManager.java:
##########
@@ -200,14 +196,7 @@ public void updateStatistics(
.build();
statisticEntities.add(statistic);
}
- TreeLockUtils.doWithTreeLock(
- identifier,
- LockType.WRITE,
- (Executable<Void, IOException>)
- () -> {
- store.batchPut(statisticEntities, true);
- return null;
- });
+ store.batchPut(statisticEntities, true);
Review Comment:
Agreed that this needs a decision. I went with the documentation route for
this PR and a separate fix for the code:
- The user-facing section of the PR description now states the case. On a
single server, a table drop that races table-scoped writes (table statistics,
and tag/policy associations, whose locks this PR also removes) waits on the
table row. Both sides lock that row first, so they queue rather than deadlock.
If the wait exceeds the backend lock-wait timeout (H2 has a short default), the
drop fails with a raw store error, as multi-node deployments already could
since #13004 and #13487.
- Translating only `TableMetaService` would special-case one path. #13776
reports lock-wait timeouts as 409 once, in the shared store exception path, for
every write. It reuses the statistic classification, and adds a real H2
lock-timeout test for a table drop. Deadlocks are out of its scope: none were
observed on these paths, and if one shows up, the lock order should be fixed
rather than mapped.
--
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]
