Skip to content

Conversation

noam-sol
Copy link

Description

This log is written on every partition, even when there's no data loss.

The position.cmp(&delta_position.from) comparison doesn't account for Position::Beginning being not equal to Position::offset(0u64). However logically speaking moving from Beginning to 0u64 doesn't indicate any data loss -

Position::Beginning means that no data was processed, and Position::offset(0)
means offset 0 was processed.

How was this PR tested?

I have run 'test_process_existing_messages' at
  quickwit/quickwit-indexing/src/source/queue_sources/coordinator.rs
and confirmed that the delta is -
  SourceCheckpointDelta[Position::offset(0)..Position::eof(file_len)]
and not -
  SourceCheckpointDelta[Position::Beginning..Position::eof(file_len)]

@noam-sol noam-sol force-pushed the suppress-log-on-beginning branch from dfa7115 to dbcc250 Compare September 28, 2025 13:54
This log is written on every partition, even when there's no data loss.

The position.cmp(&delta_position.from) comparison doesn't account for
Position::Beginning being not equal to Position::offset(0u64). However logically
speaking moving from Beginning to 0u64 doesn't indicate any data loss -

  Position::Beginning means that no data was processed, and Position::offset(0)
  means offset 0 was processed.

I have run 'test_process_existing_messages' at
  quickwit/quickwit-indexing/src/source/queue_sources/coordinator.rs
and confirmed that the delta is -
  SourceCheckpointDelta[Position::offset(0)..Position::eof(file_len)]
and not -
  SourceCheckpointDelta[Position::Beginning..Position::eof(file_len)]
@noam-sol noam-sol force-pushed the suppress-log-on-beginning branch from dbcc250 to 9d1868c Compare September 28, 2025 14:04
@guilload
Copy link
Member

This is not the right fix. It's a bug in the checkpoint delta logic in the source. If you print the batch checkpoint deltas in that unit test, you will see something like:

for batch in batches {
            println!("batch checkpoint delta: {:?}", batch.checkpoint_delta);
        }
batch checkpoint delta: ∆(file:///var/folders/cj/3nfybfc572b9s2qvd18qzh_m0000gp/T/.tmpDz99t1:(00000000000000000000..~00000000000000000350])
batch checkpoint delta: ∆(file:///var/folders/cj/3nfybfc572b9s2qvd18qzh_m0000gp/T/.tmp6o6nbz:(00000000000000000070..~00000000000000000350])

The first checkpoint delta should be (beginning..~00000000000000000350].

Do you want to take a stab at it?

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.

2 participants