Copilot commented on code in PR #12454:
URL: https://github.com/apache/gravitino/pull/12454#discussion_r3773253310
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java:
##########
@@ -126,19 +126,25 @@ public String softDeleteSchemaMetasBySchemaIds(List<Long>
schemaIds) {
}
@Override
- public String softDeleteSchemaMetasByMetalakeId(Long metalakeId) {
+ public String softDeleteSchemaMetasByCatalogId(Long catalogId) {
return "UPDATE "
+ TABLE_NAME
+ " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000
AS BIGINT)"
- + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0";
+ + " WHERE catalog_id = #{catalogId} AND deleted_at = 0";
}
+ /** {@inheritDoc} */
@Override
- public String softDeleteSchemaMetasByCatalogId(Long catalogId) {
- return "UPDATE "
+ public String softDeleteSchemaMetasWithVersion(List<SchemaPO> schemaPOs) {
+ return "<script>"
+ + "UPDATE "
+ TABLE_NAME
+ " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000
AS BIGINT)"
Review Comment:
softDeleteSchemaMetasWithVersion builds a long OR-chain over (schema_id,
current_version). For large cascades this inflates SQL size and can degrade
planning/execution. PostgreSQL supports row-value IN, which is usually more
compact and can perform better for this match pattern.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java:
##########
@@ -420,6 +422,68 @@ public boolean deleteMetalake(NameIdentifier ident,
boolean cascade) {
return true;
}
+ void deleteMetalakeWithVersion(NameIdentifier identifier, Long metalakeId,
Long currentVersion) {
+ int deleted =
+ SessionUtils.getWithoutCommit(
+ MetalakeMetaMapper.class,
+ mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId,
currentVersion));
+ if (deleted == 0) {
+ throw metalakeWriteFailure(identifier, metalakeId, identifier.name());
+ }
+ }
+
+ private RuntimeException metalakeWriteFailure(
+ NameIdentifier identifier, Long metalakeId, String observedName) {
+ MetalakePO currentMetalakePO =
+ SessionUtils.getWithoutCommit(
+ MetalakeMetaMapper.class, mapper ->
mapper.selectMetalakeMetaByIdForUpdate(metalakeId));
Review Comment:
metalakeWriteFailure() uses a SELECT ... FOR UPDATE to classify a failed
CAS. This can block behind the concurrent writer’s transaction and turn an
optimistic-lock conflict into a long wait, increasing lock contention under
load. Prefer a non-locking read here (or a NOWAIT/skip-locked variant where
supported) so conflicts fail fast.
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java:
##########
@@ -33,12 +34,18 @@ public String softDeleteCatalogMetasByCatalogId(Long
catalogId) {
+ " WHERE catalog_id = #{catalogId} AND deleted_at = 0";
}
+ /** {@inheritDoc} */
@Override
- public String softDeleteCatalogMetasByMetalakeId(Long metalakeId) {
- return "UPDATE "
+ public String softDeleteCatalogMetasWithVersion(List<CatalogPO> catalogPOs) {
+ return "<script>"
+ + "UPDATE "
+ TABLE_NAME
+ " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000
AS BIGINT)"
Review Comment:
softDeleteCatalogMetasWithVersion builds a long OR-chain over (catalog_id,
current_version). For large cascades this produces very large SQL and can lead
to poor plans. PostgreSQL supports row-value IN, which is typically more
compact and index-friendly for this kind of identifier+version match.
--
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]
