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

[OPIK-410] improve batch mechanism in SDK #648

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

alexkuzmik
Copy link
Collaborator

Details

This PR improves batching in a way that now SDK sends batched requests with the payload no bigger than 50 MB (unless one batch item already exceeds this limit).

It affects spans, feedback scores and dataset items.

Implementation details:

The batching for spans (since only spans support background batching today) could be implemented differently. We already have a batching mechanism that works inside the Streamer instance - BatchManager. This mechanism creates batches based on time intervals and the amount of accumulated items.
Memory checks were NOT added to BatchManager because they can be time-consuming and today's implementation of this class requires them to be done in the user's thread. Instead, in this PR they are performed inside the background worker threads. So, whenever the worker receives the batch created by BatchManager, it can split it additionally into smaller batches based on memory, and then send them sequentially.
An alternative approach could be to implement a new BatchManager which would work fully in the background thread and have it's own messages queue. We can switch to this approach for spans if we need it later.

Testing

Unit tests for splitting function are added.
Manual tests on big batches with heavy payloads were run.

@alexkuzmik alexkuzmik self-assigned this Nov 15, 2024
@alexkuzmik alexkuzmik requested a review from a team as a code owner November 15, 2024 15:51
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.

1 participant