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

Messages: SEEK ERROR with all partitions selected #3525

Closed
p-eye opened this issue Mar 21, 2023 · 9 comments
Closed

Messages: SEEK ERROR with all partitions selected #3525

p-eye opened this issue Mar 21, 2023 · 9 comments
Labels
scope/frontend status/accepted An issue which has passed triage and has been accepted type/bug Something isn't working
Milestone

Comments

@p-eye
Copy link
Contributor

p-eye commented Mar 21, 2023

Which version of the app are you running?

v0.6.0

Is your proposal related to a problem?

When I seek messages in topic with Offset OR Timestamp, the request is sent without a seekTo paramater.
And I got the error "seekTo should be set if seekType is OFFSET (TIMESTAMP)"

  • request without seekTo parameter
    image

  • error
    image

Describe the solution you'd like

This error occurs after the commit aeda502 (add a new condition disabling to select all partitions)
So removing the condition is a one way.

Describe alternatives you've considered

I guess your commit is for blocking long parameters with GET request.
So this is an alternative i suggest after removing above condition.

AS-IS

  • seekType: TIMESTAMP
  • seekTo: 1::1679324400000,2::1679324400000,3::1679324400000,4::1679324400000,5::1679324400000

TO-BE

  • seekType: TIMESTAMP
  • seekTo: 1679324400000
  • seekPartition: 1,2,3,4,5

Additional context

if these solutions are ok, can i develop it?

  1. only remove condition
  2. remove condition and change seekTo parameter

I would be grateful for your another suggestion.

Thank you.

@p-eye p-eye added status/triage Issues pending maintainers triage type/feature A new feature labels Mar 21, 2023
@github-actions
Copy link

Hello there p-eye! 👋

Thank you and congratulations 🎉 for opening your very first issue in this project! 💖

In case you want to claim this issue, please comment down below! We will try to get back to you as soon as we can. 👀

@p-eye
Copy link
Contributor Author

p-eye commented Mar 28, 2023

@Haarolean hi, any review?
I got #3115,
but I think only blocking all-partitions(240/240) is little bit unfair because 239/240 partitions is a still long parameter.
I cannot search messages with all partitions selected which is a default option now..

@Haarolean
Copy link
Contributor

@p-eye Hi, can you verify it works now with latest docker image?

@Haarolean Haarolean added type/bug Something isn't working status/duplicate This issue or pull request already exists status/pending Further information is requested scope/frontend and removed status/triage Issues pending maintainers triage type/feature A new feature labels Mar 28, 2023
@vcanuel
Copy link

vcanuel commented Mar 28, 2023

Hi, as mentioned in #3568 with 0.6.1 it is working with timestamp but not with offset.

@p-eye
Copy link
Contributor Author

p-eye commented Mar 29, 2023

same as vcanuel

@github-actions
Copy link

github-actions bot commented Apr 6, 2023

This issue has been automatically marked as stale because no requested feedback has been provided. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the status/stale Stale issues will be closed in 7 days. label Apr 6, 2023
@Haarolean Haarolean removed status/pending Further information is requested status/stale Stale issues will be closed in 7 days. labels Apr 6, 2023
@Haarolean
Copy link
Contributor

@p-eye

So this is an alternative i suggest after removing above condition.

We've implemented something like that within messages v2 API (#3537), but, unfortunately, that won't work with pagination because you'll have to apply all the offsets for all the partitions.

@p-eye
Copy link
Contributor Author

p-eye commented Apr 10, 2023

@Haarolean
thanks for review, hoping for other ideas 😄

@p-eye p-eye closed this as completed Apr 10, 2023
@Haarolean Haarolean reopened this Apr 10, 2023
@Haarolean Haarolean removed the status/duplicate This issue or pull request already exists label Apr 10, 2023
@Haarolean
Copy link
Contributor

offset/timestamp filtering fixed within #3568
for the fix regarding long url follow #3115

@Haarolean Haarolean added this to the 0.7 milestone Apr 10, 2023
@Haarolean Haarolean added the status/accepted An issue which has passed triage and has been accepted label Apr 10, 2023
@Haarolean Haarolean changed the title SEEK ERROR with all partitions selected Messages: SEEK ERROR with all partitions selected Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope/frontend status/accepted An issue which has passed triage and has been accepted type/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants