yuqi1129 commented on code in PR #12404:
URL: https://github.com/apache/gravitino/pull/12404#discussion_r3931794096
##########
core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java:
##########
@@ -1015,9 +1240,12 @@ public Catalog alterCatalog(NameIdentifier ident,
CatalogChange... changes)
CatalogEntity convertedCatalog =
convertFilesetCatalogEntity(updatedCatalog);
// Use put() instead of get() to force the updated wrapper into
the cache, preventing
// a background thread from overwriting it with stale data between
invalidate and put.
- CatalogWrapper newWrapper = createCatalogWrapper(convertedCatalog,
null);
- catalogCache.put(convertedCatalog.nameIdentifier(), newWrapper);
- return newWrapper.catalog();
+ try (CatalogLease lease =
+ createAndCacheCatalogLease(
Review Comment:
Addressed in 2101c93641. `alterCatalog` now takes the lifecycle read lock
before `alterCatalogUnderLock` and holds it through cache publication, so
`close()` can no longer land between the persisted change and
`createAndCacheCatalogLease`'s `checkOpen()`. Lock ordering is unchanged (tree
lock first, lifecycle lock second), and the reentrant read acquisition inside
`createAndCacheCatalogLease` is safe even with a writer queued.
Added `testManagerCloseWaitsForConcurrentCatalogAlter`, which pins the
publication window open with a latch and asserts that `close()` blocks and the
alter returns the updated catalog; without the fix the "close is blocked"
assertion fails.
--
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]
