hubcio commented on code in PR #4146:
URL: https://github.com/apache/iggy/pull/4146#discussion_r3996803031


##########
core/journal/src/partition_journal.rs:
##########
@@ -234,27 +269,121 @@ impl<S: DurableStorage> PartitionPrepareJournal<S> {
             preallocate_segments,
             retained_bytes: 0,
         };
-        journal.recover_entries().await?;
-        // Only bytes covered by the durable frontier could have released an 
ack.
-        journal.file.truncate(state.length).await?;
+        // A slot that did not verify may have been the newest, so the frontier
+        // this open read can name less than an acknowledgment already covered.
+        // Only then does recovery walk past the frontier.
+        journal.recover_entries(verified < slots).await?;
+        // Only verified bytes could have released an ack. Recovery adopted 
every
+        // record past the published frontier whose envelope, chain and body 
all
+        // verify; what follows them is a tail no barrier ever covered.
+        let recovered = journal.state;
+        journal.durable_head = recovered.head;
+        journal.file.truncate(recovered.length).await?;
         journal.file.sync().await?;
         journal.storage.sync_directory(directory).await?;
-        if existing.is_none() {
-            journal.publish(state).await?;
-        }
-        journal.discover_obsolete().await?;
-        loop {
-            let remaining = journal.obsolete.len();
-            journal.cleanup_obsolete().await;
-            if journal.obsolete.is_empty() || journal.obsolete.len() == 
remaining {
-                break;
-            }
+        if recovered != state {

Review Comment:
   warning: when `recovered == state`, a damaged frontier slot survives reopen 
and can make a later partial, unacknowledged tail prevent recovery. republish 
whenever `verified < slots`, even when the recovered state is unchanged.



##########
core/journal/src/partition_journal.rs:
##########
@@ -790,9 +933,110 @@ impl<S: DurableStorage> PartitionPrepareJournal<S> {
             .values()
             .map(|entry| entry.retained_bytes)
             .sum();
+        if lost_publication {
+            self.recover_unpublished_tail(previous, checksum).await
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Recover the records a frontier publication this open could not read had
+    /// already covered.
+    ///
+    /// Runs only when a slot failed to verify, which can happen only while a
+    /// publication was in flight. The writer is serial and publishes after 
both
+    /// data barriers, so a publication in flight proves every record before it
+    /// was already durable: between the surviving frontier and the end of the
+    /// data file the records are complete, and every one of them is adopted.
+    ///
+    /// That is why this walk REFUSES instead of stopping. Once a slot is lost,
+    /// nothing left on disk says how far acknowledgment had reached, so a 
record
+    /// that does not verify there cannot be dismissed as an unwritten tail. It
+    /// is damage to history that may have been acknowledged, and the partition
+    /// has to fence and rebuild from its peers rather than open a truncated 
log.
+    /// When both slots verify the frontier is exact and the tail is discarded 
as
+    /// it always was.
+    async fn recover_unpublished_tail(
+        &mut self,
+        mut previous: u64,
+        mut checksum: u128,
+    ) -> io::Result<()> {
+        let limit = self.file.length().await?;
+        if limit > self.capacity {

Review Comment:
   warning: lowering WAL capacity can make valid history fail to reopen when a 
frontier slot is damaged. remove both admission-capacity checks from recovery 
while keeping the file and protocol bounds.



-- 
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]

Reply via email to