Skip to content

reverify append on SeqNumMismatch via read-back#15

Merged
quettabit merged 1 commit intomainfrom
qb/msn
Feb 19, 2026
Merged

reverify append on SeqNumMismatch via read-back#15
quettabit merged 1 commit intomainfrom
qb/msn

Conversation

@quettabit
Copy link
Member

No description provided.

@greptile-apps
Copy link

greptile-apps bot commented Feb 19, 2026

Greptile Summary

Updated s2-sdk to version 0.24.2 and added reverification logic for append operations. When an append with match_seq_num fails with SeqNumMismatch, the code now reads back records from the stream to determine if the append actually succeeded (e.g., due to a retry after connection break).

Key changes:

  • New reverify_append function reads back from match_seq_num to verify if records were actually appended
  • Clones AppendRecordBatch when match_seq_num is set to enable reverification
  • Compares both body and headers of appended vs. read-back records
  • Returns AppendIndefiniteFailure if read-back fails, AppendDefiniteFailure if records don't match, AppendSuccess if they do
  • Dependency updates including removal of unused dependencies (reqwest, quinn-related crates)

Confidence Score: 4/5

  • This PR is safe to merge with minor risk - the reverification logic is sound but adds complexity
  • The core logic is correct: when SeqNumMismatch occurs with match_seq_num, the code reads back to verify if the append actually succeeded. The implementation properly compares both body and headers, handles read failures gracefully, and correctly classifies outcomes. However, cloning AppendRecordBatch on every conditional append adds memory overhead, and the edge case where records are partially written (though unlikely) could theoretically cause issues. The dependency updates are straightforward and safe.
  • No files require special attention

Important Files Changed

Filename Overview
rust/s2-verification/src/history.rs Added reverify_append function to handle SeqNumMismatch errors by reading back records to check if append succeeded; clones AppendRecordBatch when match_seq_num is set

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Append with match_seq_num] --> B{Append Result}
    B -->|Success| C[Return AppendSuccess]
    B -->|SeqNumMismatch Error| D[reverify_append]
    B -->|Other AppendConditionFailed| E[Return AppendDefiniteFailure]
    B -->|Other Errors| F[Return AppendIndefiniteFailure]
    
    D --> G[Read back from match_seq_num]
    G -->|Read Failed| F
    G -->|Read Success| H{Record Count Match?}
    
    H -->|No| E
    H -->|Yes| I{Compare Body & Headers}
    
    I -->|Match| J[Return AppendSuccess with tail]
    I -->|Mismatch| E
Loading

Last reviewed commit: e56458d

@quettabit quettabit merged commit 47a8ad8 into main Feb 19, 2026
3 checks passed
quettabit added a commit that referenced this pull request Feb 19, 2026
quettabit added a commit that referenced this pull request Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant