github-actions[bot] commented on code in PR #69633:
URL: https://github.com/apache/doris/pull/69633#discussion_r4398402493
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java:
##########
@@ -510,14 +535,72 @@ public void renameTable(String dbName, String oldName,
String newName) {
@Override
public void setTableProperties(String dbName, String tableName,
Map<String, String> properties) {
+ TableIdentifier identifier = toTableIdentifier(dbName, tableName);
withTable(dbName, tableName, table -> {
- UpdateProperties updateProperties = table.updateProperties();
- properties.forEach(updateProperties::set);
- updateProperties.commit();
+ commitTableProperties(identifier, table, properties);
return null;
});
}
+ /**
+ * Applies the property map with Iceberg's optimistic commit contract
while validating every metadata base
+ * that can actually be committed. {@link Table#updateProperties()}
refreshes inside its retry loop, so a
+ * one-time precheck can validate a different schema from the one
eventually upgraded.
+ */
+ private void commitTableProperties(TableIdentifier identifier, Table
table, Map<String, String> properties) {
+ if (!(table instanceof HasTableOperations)) {
+ throw new DorisConnectorException(
+ "Cannot update properties for an Iceberg table without
metadata operations: " + table.name());
+ }
+
+ TableOperations operations = ((HasTableOperations)
table).operations();
+ TableMetadata current = operations.current();
+ Tasks.foreach(operations)
+
.retry(current.propertyTryAsInt(TableProperties.COMMIT_NUM_RETRIES,
+ TableProperties.COMMIT_NUM_RETRIES_DEFAULT))
+ .exponentialBackoff(
+
current.propertyTryAsInt(TableProperties.COMMIT_MIN_RETRY_WAIT_MS,
+
TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT),
+
current.propertyTryAsInt(TableProperties.COMMIT_MAX_RETRY_WAIT_MS,
+
TableProperties.COMMIT_MAX_RETRY_WAIT_MS_DEFAULT),
+
current.propertyTryAsInt(TableProperties.COMMIT_TOTAL_RETRY_TIME_MS,
+
TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT),
+ 2.0)
+ .onlyRetryOn(CommitFailedException.class)
+ .run(taskOps -> {
+ TableMetadata base = taskOps.refresh();
+ Map<String, String> updatedProperties = new
HashMap<>(base.properties());
+ updatedProperties.putAll(properties);
+ MetricsConfig.validate(updatedProperties,
base.schema());
+
IcebergSchemaUtils.validateRowLineageFormatVersionUpgrade(base, properties);
+ TableMetadata updated =
base.replaceProperties(updatedProperties);
+ boolean requiresSchemaFence =
+
IcebergSchemaUtils.requiresRowLineageFormatVersionUpgrade(base, properties);
+ if (requiresSchemaFence && restTableCommit != null) {
+ commitSchemaFencedRestUpgrade(identifier, base,
updated);
+ } else if (requiresSchemaFence && restFlavor) {
+ throw new DorisConnectorException(
+ "Cannot safely upgrade a wrapped Iceberg
REST catalog to v3");
+ } else {
+ taskOps.commit(base, updated);
+ }
+ });
+ }
+
+ /**
+ * REST servers can rebase property updates onto a newer metadata
version without returning a conflict.
+ * Bind a row-lineage format upgrade to the exact schema generation
that was validated on the client.
+ */
+ private void commitSchemaFencedRestUpgrade(
+ TableIdentifier identifier, TableMetadata base, TableMetadata
updated) {
+ TableCommit tableCommit = TableCommit.create(identifier, base,
updated);
+ TableCommit fencedCommit = ImmutableTableCommit.builder()
+ .from(tableCommit)
+ .addRequirements(new
UpdateRequirement.AssertCurrentSchemaID(base.currentSchemaId()))
+ .build();
+ restTableCommit.commit(Collections.singletonList(fencedCommit));
Review Comment:
[P2] Preserve REST upgrades without the optional transaction endpoint
For a V3-capable REST server whose `/v1/config` advertises table load/update
but omits `POST /v1/{prefix}/transactions/commit`, upgrading a clean V2 table
with `ALTER TABLE ... SET ("format-version" = "3")` now always fails. Both
production callbacks route this single-table change through
`RESTSessionCatalog.commitTransaction`, whose endpoint check throws
`UnsupportedOperationException` before committing. Iceberg explicitly makes
[multi-table commit
optional](https://iceberg.apache.org/docs/latest/rest-protocol/#commit-protocol),
so table-update support does not imply this endpoint exists. Previously the
update used the supported single-table endpoint. Preserve
`AssertCurrentSchemaID` while submitting through the single-table update route,
and cover a REST client whose advertised endpoints exclude transaction commit;
the current fake commit callback bypasses that check.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
