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

Gracefully handle empty task list #37

Merged
merged 5 commits into from
Nov 27, 2024
Merged

Gracefully handle empty task list #37

merged 5 commits into from
Nov 27, 2024

Conversation

lgarrison
Copy link
Member

Currently, if the task list is empty, the KVS server will shut down before the compute context has a chance to connect, resulting in an exception. If we know we have an empty task list, then it seems sensible to never start the compute context. This is what this PR implements.

Knowing if we have an empty task list is a little subtle. We need to parse the task list, e.g. to expand any repeats, to know if the list is actually empty. And the task list can be any iterable, including a generator, which seems like a nice property to preserve. So this PR essentially implements a "peeking" generator that looks one item ahead, letting us know if the task list is exhausted. If so, we set a property on the generator that calling code can query.

Fixes #36.

@lgarrison lgarrison requested a review from njcarriero November 21, 2024 00:43
@njcarriero
Copy link
Collaborator

What happens if the task source is dynamic and slow to generate the first task? Will the whole start up be frozen at 2421 when the TaskGenerator object is instantiated?
Somewhat related, if it never generates a task, would disBatch block here? Perhaps it might be useful to add a log message something like "Looking for first task, could block."?

@lgarrison
Copy link
Member Author

Will the whole start up be frozen at 2421 when the TaskGenerator object is instantiated?

Yes, I think that's right, which is somewhat unfortunate. In most cases, though, I think this just means that it would block before startup rather than after, so not much difference from the user's point of view.

One way to avoid this would be to covert disBatch to an async event loop/coroutine model, which I think Dylan has mentioned before. But that would be a much bigger change! For now, I'll just add more logging.

@lgarrison lgarrison merged commit 591d537 into master Nov 27, 2024
7 checks passed
@lgarrison lgarrison deleted the empty-tasks branch November 27, 2024 21:17
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.

Handle empty task list better
2 participants