yuqi1129 commented on code in PR #12168:
URL: https://github.com/apache/gravitino/pull/12168#discussion_r3683255003
##########
catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java:
##########
@@ -564,27 +564,18 @@ 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} whose message starts with {@link
- * TableMetaService#UPDATE_ENTITY_CONFLICT_MESSAGE_PREFIX}. Because the
updater is idempotent, the
- * loser sleeps a short randomized backoff (to avoid re-colliding), re-reads
the latest (already
- * repaired) entity, and retries instead of failing the whole load with a
fatal error. Other IO
- * failures (DB outage, serialization errors, etc.) are not conflicts and
fail fast.
+ * an {@link OptimisticLockException}. Because the updater is idempotent,
the loser sleeps a short
+ * randomized backoff (to avoid re-colliding), re-reads the latest (already
repaired) entity, and
+ * retries instead of failing the whole load with a fatal error. Other IO
failures (DB outage,
+ * serialization errors, etc.) are not conflicts and fail fast.
*/
private TableEntity updateTableWithCasRetry(
NameIdentifier ident, Function<TableEntity, TableEntity> updater) throws
IOException {
- IOException lastConflict = null;
+ OptimisticLockException 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) {
Review Comment:
Good question. The retry behavior in Lance predates this PR and was
introduced to fix #11891/#11892.
LanceTableOperations.loadTable may perform an internal repair-on-load
update. When multiple clients load the same table concurrently, they can read
the same metadata version and attempt the same repair. One update wins the CAS,
while the others receive an optimistic-lock conflict even though the metadata
has already been repaired correctly.
Retrying is safe here because the repair updater is idempotent and each
retry re-reads the latest entity before reapplying the update. It also prevents
an ordinary table load from failing because of an internal metadata-repair race.
We should not retry every optimistic-lock conflict globally. A retry is
appropriate only when the operation is idempotent, is recomputed from the
latest state, has no non-idempotent external side effects, and uses a
bounded retry/backoff policy. User-initiated alter operations generally do
not meet these conditions, so we return HTTP 409 and let the caller reload the
latest state and decide whether to retry. Other internal operations can use the
same approach if they satisfy these criteria
--
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]
