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

persist: in persist_source, do very fine-grained output activation #16390

Conversation

aljoscha
Copy link
Contributor

Previously, if there where too many parts at the same timestamp we would be holding activated output handles and sessions across await points for too long, which can prevent flushing of the shared timely output buffer.

Previously, if there where too many parts at the same timestamp we would
be holding activated output handles and sessions across await points for
too long, which can prevent flushing of the shared timely output buffer.
Copy link
Contributor

@petrosagg petrosagg left a comment

Choose a reason for hiding this comment

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

Looks good! I think the previous version was also never truly yielding correctly because everything was buffered in the activated output handle.

Comment on lines 429 to 436
// Do very fine-grained output activation/session
// creation to ensure that we don't hold activated
// outputs or sessions across await points, which
// would prevent messages from being flushed from
// the shared timely output buffer.
let mut output_handle = update_output.activate();
let mut update_session = output_handle.session(&cap);

Copy link
Contributor

Choose a reason for hiding this comment

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

this is still held across a yield_now await point below, you probably need to open a new { } around these + the give_vec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! 🙈

@aljoscha aljoscha merged commit 0bcb204 into MaterializeInc:main Dec 1, 2022
@aljoscha aljoscha deleted the persist-source-fine-grained-output-activation branch December 1, 2022 13:43
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