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

feat: replace all objects with loaded objects from a Sequence #400

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KengoTODA
Copy link

Hello, thanks for keeping this client maintained and open! 🙌
I'm using this client to search and maintain data, and here is a PR that suggests providing more support to lazy programmers:

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related Issue nothing
Need Doc update no

Describe your change

Support not only the List interface but also the Sequence interface when replacing all objects in an index.

What problem is this fixing?

Currently the replaceAllObjects() supports the List interface only, which means that callers need to load all data onto the Java heap even though it could be huge.

By supporting the Sequence interface, callers can generate data lazily and reduce the memory footprint of their code.

Signed-off-by: Kengo TODA <skypencil@gmail.com>
@KengoTODA KengoTODA requested a review from aallam as a code owner July 11, 2023 20:35

return mutableListOf<TaskIndex>().also { list ->
list += TaskIndex(indexName, indexSource.copyIndex(indexDestination.indexName, scopes).taskID)
batchOperations.chunked(10_000).forEach {
Copy link
Author

Choose a reason for hiding this comment

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

It would be difficult to find the "best number" that suits every case in the world.

At least in our case 10_000 works fine, but please tell me if we can improve this implementation (e.g. making it configurable?).

Signed-off-by: Kengo TODA <skypencil@gmail.com>
@KengoTODA KengoTODA changed the title feat: replace all objects loaded from a Sequence feat: replace all objects with loaded objects from a Sequence Jul 13, 2023
@KengoTODA
Copy link
Author

Hello @aallam, could you review this PR when you have time? Thanks!

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.

None yet

1 participant