zhoujinsong commented on code in PR #4283:
URL: https://github.com/apache/amoro/pull/4283#discussion_r3628650231
##########
amoro-ams/src/main/java/org/apache/amoro/server/catalog/InternalCatalog.java:
##########
@@ -56,6 +59,12 @@ public List<String> listDatabases() {
}
public void createDatabase(String databaseName) {
+ createDatabase(databaseName, Collections.emptyMap());
+ }
+
+ public void createDatabase(String databaseName, Map<String, String>
properties) {
+ Map<String, String> databaseProperties =
+ properties == null ? Collections.emptyMap() : new
HashMap<>(properties);
if (!databaseExists(databaseName)) {
doAsTransaction(
Review Comment:
The `lockDatabase` + `selectDatabaseMetadata` + `updateDatabaseProperties`
sequence uses pessimistic locking to serialize concurrent property updates, but
the current implementation does not achieve that goal.
The Derby integration test passes because Derby's table-level locking is
coarse enough to mask this, but MySQL (InnoDB) and PostgreSQL would not
serialize correctly under this pattern.
A more standard approach is to use `SELECT ... FOR UPDATE` to combine lock +
read in a single statement, and execute the entire read-then-write sequence
within one `SqlSession`:
```java
try (NestedSqlSession session = beginSession()) {
TableMetaMapper mapper = getMapper(session, TableMetaMapper.class);
// 1. SELECT FOR UPDATE: lock + read atomically
DatabaseMetadata metadata =
mapper.selectDatabaseMetadataForUpdate(name(), databaseName);
// 2. merge in memory
Map<String, String> properties = new HashMap<>(metadata.getProperties());
requestedRemovals.forEach(properties::remove);
properties.putAll(requestedUpdates);
// 3. UPDATE
mapper.updateDatabaseProperties(name(), databaseName, properties);
session.commit();
return metadata.getProperties(); // previous state for the REST response
}
```
This eliminates the dummy `UPDATE table_count = table_count` hack, reduces
DB round-trips, and works correctly across all supported databases.
--
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]
