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): improving search and listing due to higher timeout and max.poll.records #1843

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

jonasvoelcker
Copy link
Contributor

fixes #1834

@AlexisSouquiere
Copy link
Collaborator

AlexisSouquiere commented Jun 20, 2024

I think we should add the matchesCount increment and break in the records loop (like in the search function) in the consumeOldest and consumeNewest functions to prevent iterating over the possible 25K records if we have our topic-data.size number of messages returned by the poll.

It will replace this

that is done too late for me. I quickly tested locally and by doing that I'm improving the iteration number from 3k (number of records given by poll) to 50 (because there is no filters applied in my case so I just want the fist 50 records)

WDYT ?

@jonasvoelcker
Copy link
Contributor Author

jonasvoelcker commented Jun 20, 2024

Hi @AlexisSouquiere,

maybe we could override the max.poll.records for the normal listings with the page size, I'll test it tomorrow whether this leads to a valid pagination or not ;)

@jonasvoelcker
Copy link
Contributor Author

Hey @AlexisSouquiere,

I tested a lot and it is enough to use the page-size as max.poll.records. I also left the limit inside the stream api in place so there is no way to output more entries then desired. The "after" gets calculated in the Controller not the RecordRepository so the limit also effects the pagination.

@AlexisSouquiere
Copy link
Collaborator

AlexisSouquiere commented Jun 21, 2024

Sounds good ! 👌

There is one thing that I figured out during the tests that I will address in another PR. But it's not linked to these changes. Oldest sort can show different messages if you refresh the page several times and maybe don't show the real oldest messages. We are using a single consumer with all the partitions assigned and depending on which partition answers first, we will show these messages. But one the same request, the next time another partition can answer faster so it will show different messages. Order is guaranteed in a partition but not accros partition. I started to switch from a single consumer with all the partition assigned to a consumer by partition to get to "real" oldest messages.

But as I said it's not linked to this PR !

@jonasvoelcker
Copy link
Contributor Author

Hey @AlexisSouquiere,

I can also see the same problem on my test topic, that's really creepy :(

@AlexisSouquiere AlexisSouquiere self-requested a review June 21, 2024 10:12
@jonasvoelcker
Copy link
Contributor Author

@AlexisSouquiere That's weird, after my last change there is a newly created topic present ^^

image

@jonasvoelcker
Copy link
Contributor Author

Hi @tchiotludo, @AlexisSouquiere,

do you plan to provide a 0.25.1 anytime soon? Unfortunately we are still kind of blocked in our searches.

Best Regards
Jonas

@tchiotludo tchiotludo merged commit 1235dee into tchiotludo:dev Jun 27, 2024
4 checks passed
@tchiotludo
Copy link
Owner

yes this is the plan to release before my holidays end of next week 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Searching messages is currently ridiculously slow
3 participants