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(server): fix get_messages_by_timestamp logic #1560

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Conversation

hubcio
Copy link
Contributor

@hubcio hubcio commented Feb 19, 2025

This commit refactors the get_messages_by_timestamp function
to improve efficiency and readability. The changes include:

  • Simplified logic for finding the starting segment and offset.
  • Removed unnecessary calculations and variables.
  • Enhanced error handling with more detailed context.
  • Improved message filtering and collection logic.
  • Removed redundant functions and code for better maintainability.

Besides that, added additional 288 testcases that test both
get_messages_by_timestamp and get_messages_by_offset to improve
robustness of iggy-server.

@hubcio hubcio force-pushed the fix-get-by-timestamp branch 2 times, most recently from 53ca5fb to 62470c8 Compare February 20, 2025 00:54
This commit refactors the `get_messages_by_timestamp` function
to improve efficiency and readability. The changes include:

- Simplified logic for finding the starting segment and offset.
- Removed unnecessary calculations and variables.
- Enhanced error handling with more detailed context.
- Improved message filtering and collection logic.
- Removed redundant functions and code for better maintainability.

Besides that, added additional 288 testcases that test both
`get_messages_by_timestamp` and `get_messages_by_offset` to improve
robustness of iggy-server.
@hubcio hubcio force-pushed the fix-get-by-timestamp branch from 62470c8 to e3e65b7 Compare February 20, 2025 11:40
@hubcio hubcio merged commit 7c7f2e5 into master Feb 20, 2025
14 checks passed
@hubcio hubcio deleted the fix-get-by-timestamp branch February 20, 2025 11:57
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