Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resume from reader cursor when no senders configured #167

Merged

Conversation

ngilles-aiven
Copy link
Contributor

In cases where a reader was configured with no senders (eg. only configured with searches), then in case of reloads/restarts the resume cursor was assumed None, and the default 'initial_position' of 'head' is fallen back to. This would end up causing all matching searchg to resend metric events immediately.

In cases where a reader was configured with no senders (eg. only
configured with searches), then in case of reloads/restarts the resume
cursor was assumed `None`, and the default 'initial_position' of 'head'
is fallen back to. This would end up causing all matching searchg to
resend metric events immediately.
@ngilles-aiven ngilles-aiven force-pushed the ngilles/resume-from-read-cursor-when-no-senders branch from 14d8214 to 7d01dac Compare March 15, 2024 07:29
@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.23%. Comparing base (3f80062) to head (7d01dac).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
- Coverage   63.61%   63.23%   -0.39%     
==========================================
  Files          19       19              
  Lines        2086     2086              
==========================================
- Hits         1327     1319       -8     
- Misses        759      767       +8     
Files Coverage Δ
journalpump/journalpump.py 63.59% <100.00%> (+0.55%) ⬆️

... and 4 files with indirect coverage changes

Copy link
Contributor

@aiven-amartin aiven-amartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM! Would be good to tackle the TODO around oldest sender, but that can come later.

@aiven-amartin aiven-amartin merged commit 5cf281e into master Mar 15, 2024
6 checks passed
@aiven-amartin aiven-amartin deleted the ngilles/resume-from-read-cursor-when-no-senders branch March 15, 2024 13:50
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.

3 participants