Copilot commented on code in PR #11892:
URL: https://github.com/apache/gravitino/pull/11892#discussion_r3519538328
##########
catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java:
##########
@@ -547,13 +552,38 @@ private Table repairTableMetadata(NameIdentifier ident,
Column[] columns, long d
}
}
+ /**
+ * Applies an idempotent update to the stored table, retrying when the
optimistic-lock CAS is lost
+ * to a concurrent update. The repair-on-load path runs on every {@code
loadTable}, so concurrent
+ * loads of the same table race on the version CAS; {@code store.update}
surfaces the lost race as
+ * an {@link IOException}. Because the updater is idempotent, the loser
re-reads the latest
+ * (already repaired) entity and retries instead of failing the whole load
with a fatal error.
+ */
+ private TableEntity updateTableWithCasRetry(
+ NameIdentifier ident, Function<TableEntity, TableEntity> updater) throws
IOException {
+ IOException lastConflict = null;
+ for (int attempt = 1; attempt <= REPAIR_UPDATE_MAX_ATTEMPTS; attempt++) {
+ try {
+ return store.update(ident, TableEntity.class, Entity.EntityType.TABLE,
updater);
+ } catch (IOException e) {
+ lastConflict = e;
+ LOG.debug(
+ "Optimistic-lock conflict updating table {} metadata (attempt
{}/{}), {}",
+ ident,
+ attempt,
+ REPAIR_UPDATE_MAX_ATTEMPTS,
+ attempt < REPAIR_UPDATE_MAX_ATTEMPTS ? "retrying" : "retries
exhausted",
+ e);
+ }
Review Comment:
`updateTableWithCasRetry` currently treats *any* `IOException` from
`store.update(...)` as an optimistic-lock CAS conflict: it retries up to 5
times and logs "Optimistic-lock conflict...". Per `EntityStore.update`'s
contract, `IOException` can also represent real storage failures (DB
connectivity, serialization errors, etc.), where retrying here is unlikely to
help and the log message becomes misleading. Consider retrying only for the
known CAS-loss case (currently surfaced as `IOException("Failed to update the
entity: ...")`), and fail fast for other `IOException`s.
--
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]
