github-actions[bot] commented on code in PR #4392:
URL: https://github.com/apache/iggy/pull/4392#discussion_r4180852838
##########
core/partitions/src/iggy_partition.rs:
##########
@@ -8022,38 +7981,35 @@ where
}
// Directory fsync so those unlinks stick, mirroring the install path:
a
// crash right after the purge otherwise resurrects the offset files at
- // boot, and while recovery clamps a resurrected offset down to the
- // rebuilt head, "consumed through 0" is not the intended "no entry at
- // all" -- that consumer skips the first post-purge message. Logged on
- // failure, sharper than the partition-dir fsync above: the generation
- // write below still runs, so a crash would resurrect these files with
- // no boot re-purge left to clear them.
- for dir in self
- .consumer_offsets_path
- .clone()
- .into_iter()
- .chain(self.consumer_group_offsets_path.clone())
- {
- if let Err(error) = storage.sync_directory(Path::new(&dir)).await {
+ // boot. Recovery clamps a resurrected Iggy offset down to the rebuilt
+ // head, but "consumed through 0" is not the intended "no entry at all"
+ // -- that consumer skips the first post-purge message -- and boot
never
+ // clamps an external group offset, so one of 1000 skips 1000 messages.
+ // A failed sync therefore leaves the generation unrecorded below, and
+ // the reconciler re-issues the purge.
+ let mut offset_dirs_synced = true;
+ for dir in self.consumer_offset_dirs.iter().flatten() {
+ if let Err(error) = storage.sync_directory(Path::new(dir)).await {
Review Comment:
warning: When the offsets directory is already gone, `sync_directory`
returns `NotFound`, which the purge counts as a failed sync, so every re-issued
purge fails and the replica withholds `PrepareOk` for that partition. Tolerate
`ErrorKind::NotFound` here, as `flush_consumer_offset_directories_for` does.
##########
core/partitions/src/state_transfer.rs:
##########
@@ -403,9 +409,12 @@ impl ConsumerOffsetsWire {
let next_offset = cursor.u64()?;
let consumer_count = cursor.u32()?;
let group_count = cursor.u32()?;
+ let external_group_count = cursor.u32()?;
Review Comment:
warning: `decode` reads a fourth count field that an older build's artifact
does not carry, but `CONSUMER_OFFSETS_VERSION` stays 1, so the version guard
accepts both layouts and a mixed-version peer misparses the artifact. Bump the
version to 2.
--
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]
