You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "snazy (via GitHub)" <gi...@apache.org> on 2026/10/08 08:34:09 UTC

[PR] NoSQL: Preserve unresolved object writes across commit retries [polaris]

snazy opened a new pull request, #5741:
URL: https://github.com/apache/polaris/pull/5741

   `CommitterImpl` cleared pending writes before `writeMany()` succeeded, while `writeIfNew()` continued to reuse cached objects. After an ambiguous partial write failure, a retry could publish a head referencing objects that had never been persisted.
   
   Retain unresolved writes separately, merge them into subsequent batches, and clear them only after `writeMany()` succeeds. Keep the current head last so the persisted result corresponds to the head being published.
   
   Add regression coverage for partial and repeated write failures with both fresh and reused heads. Verify that reused children are persisted before the reference is updated.
   


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NoSQL: Preserve unresolved object writes across commit retries [polaris]

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5741:
URL: https://github.com/apache/polaris/pull/5741#discussion_r4228897744


##########
persistence/nosql/persistence/impl/src/main/java/org/apache/polaris/persistence/nosql/impl/commits/CommitterImpl.java:
##########
@@ -491,10 +492,15 @@ Optional<RESULT> commitAttempt(
               + referenceType.getName());
     }
 
-    state.forAttempt.put(resultObjRef, resultObj);
-    var objs = state.forAttempt.values().toArray(new Obj[0]);
+    state.previousAttemptWrites.putAll(state.forAttempt);
+    // Keep the current head last, including when it was already in an unsuccessful write batch.
+    state.previousAttemptWrites.remove(resultObjRef);
+    state.previousAttemptWrites.put(resultObjRef, resultObj);
+    var objs = state.previousAttemptWrites.values().toArray(new Obj[0]);

Review Comment:
   `previousAttemptWrites` retains the abandoned attempt's result object along with its children, so the head of every failed attempt is written in a later batch and then left behind. With two ambiguous write failures and a fresh head per attempt, the final successful batch is four objects: the child, the new head, and two commit objects that nothing will ever reference. Both land in `allPersistedIds`, and `deleteIds` is only populated on failure, so a successful commit leaves them persisted. Previously an abandoned head was often never written at all, so this turns it into a guaranteed unreachable object.
   
   Retaining only the non-result objects avoids it, and the rest of the behaviour stays unchanged, keeping `previousAttemptWrites` free of `resultObjRef` and building the array as its values followed by `resultObj`:
   
   ```
   state.previousAttemptWrites.remove(resultObjRef);
   var pending = state.previousAttemptWrites.values();
   var objs = pending.toArray(new Obj[pending.size() + 1]);
   objs[objs.length - 1] = resultObj;
   ```



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NoSQL: Preserve unresolved object writes across commit retries [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #5741:
URL: https://github.com/apache/polaris/pull/5741


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NoSQL: Preserve unresolved object writes across commit retries [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #5741:
URL: https://github.com/apache/polaris/pull/5741#discussion_r4241860635


##########
persistence/nosql/persistence/impl/src/main/java/org/apache/polaris/persistence/nosql/impl/commits/CommitterImpl.java:
##########
@@ -491,10 +492,15 @@ Optional<RESULT> commitAttempt(
               + referenceType.getName());
     }
 
-    state.forAttempt.put(resultObjRef, resultObj);
-    var objs = state.forAttempt.values().toArray(new Obj[0]);
+    state.previousAttemptWrites.putAll(state.forAttempt);
+    // Keep the current head last, including when it was already in an unsuccessful write batch.
+    state.previousAttemptWrites.remove(resultObjRef);
+    state.previousAttemptWrites.put(resultObjRef, resultObj);
+    var objs = state.previousAttemptWrites.values().toArray(new Obj[0]);

Review Comment:
   Good point!



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org