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

Dynamic EventPage Rechunking #57

Merged
merged 2 commits into from
Apr 9, 2024
Merged

Conversation

cjtitus
Copy link
Contributor

@cjtitus cjtitus commented Mar 6, 2024

Summary

Added try/except to Publisher to handle case where EventPages are too large, and dynamically re-chunk them.

Motivation

When flyscanning, we can produce quite large EventPages of data that cause the Kafka producer to choke. Fortunately, an EventPage can be re-chunked quite easily (once bluesky/event-model#297 is merged), so we should take advantage of this to produce documents that Kafka can handle.

Details

I have added a try/except to the Publisher to catch the KafkaException indicating that a message is too large. If the message is too large, and the document was an EventPage, we should try re-chunking the document to half its size, and re-sending.

I have made this process recursive, so that the page size will be halved until the messages are small enough. Hopefully this only needs to happen once or twice, but with fly-scans, quite large EventPages can be generated.

Testing

I have tested this by publishing quite large test pages, which successfully publish. All existing tests pass. Possibly more tests could be implemented.

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. Thanks, @cjtitus!

Prevent potential infinite recursion by checking page_len

Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>
@cjtitus
Copy link
Contributor Author

cjtitus commented Apr 9, 2024

@tacaswell does everything look good now that I added a check for infinite recursion? I'd like to get this merged sooner rather than later, as we want to really begin serious fly-scanning next cycle.

@tacaswell tacaswell merged commit 2aaf2e1 into bluesky:main Apr 9, 2024
3 checks passed
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.

3 participants