Copilot commented on code in PR #13737:
URL: https://github.com/apache/gravitino/pull/13737#discussion_r4236319100
##########
core/src/test/java/org/apache/gravitino/catalog/TestPartitionOperationDispatcher.java:
##########
@@ -161,6 +162,35 @@ public void testDropPartition() {
partitionOperationDispatcher.partitionExists(TABLE_IDENT,
testDrop.name()));
}
+ @Test
+ public void testPartitionOperationsDoNotWaitForTableTreeLock() throws
Exception {
+ Partition partition =
+ Partitions.identity(
+ "p_lock",
+ new String[][] {{"col1"}},
+ new Literal[] {Literals.stringLiteral("v_lock")},
+ Maps.newHashMap());
+
+ // Partition operations never write the entity store, so they must not
contend with an
+ // in-flight table operation that holds the table tree lock.
+ try (TreeLockTestSupport.HeldLock tableWrite =
+ TreeLockTestSupport.HeldLock.acquire(TABLE_IDENT, LockType.WRITE)) {
+ TreeLockTestSupport.assertRunsConcurrentlyWith(
+ tableWrite, () ->
partitionOperationDispatcher.addPartition(TABLE_IDENT, partition));
+ TreeLockTestSupport.assertRunsConcurrentlyWith(
+ tableWrite,
+ () -> partitionOperationDispatcher.getPartition(TABLE_IDENT,
partition.name()));
+ TreeLockTestSupport.assertRunsConcurrentlyWith(
+ tableWrite, () ->
partitionOperationDispatcher.listPartitionNames(TABLE_IDENT));
+ TreeLockTestSupport.assertRunsConcurrentlyWith(
+ tableWrite,
+ () -> partitionOperationDispatcher.dropPartition(TABLE_IDENT,
partition.name()));
Review Comment:
This test verifies only four of the six lock removals: `listPartitions` and
`purgePartition` are never exercised while the table WRITE lock is held. Add
both calls here so a future reintroduction of either removed lock is detected.
##########
docs/gravitino-server-config.md:
##########
@@ -348,8 +348,10 @@ in memory, so raise `pollBatchSize` in moderate steps.
#### Tree Lock
-Gravitino serializes conflicting metadata operations with an in-memory tree
lock. It is the only
-lock implementation available, and it is per-server.
+Gravitino serializes some conflicting metadata operations with an in-memory
tree lock. It is the
+only lock implementation available, and it is per-server. Partition, tag,
policy and owner
+operations do not take it: the entity store enforces their consistency, and a
concurrent alter or
+drop of the same tag or policy is rejected with HTTP 409 instead of waiting.
Review Comment:
This overstates the conflict contract. Concurrent deletes are intentionally
idempotent and return `false` when the other delete/rename wins
(`TagMetaService.deleteTag` and `PolicyMetaService.deletePolicy`), so a
concurrent drop is not necessarily HTTP 409. Describe 409 as a possible result
of conflicting writes rather than a guaranteed alter/drop result.
--
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]
