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

New task log API #26

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

New task log API #26

wants to merge 2 commits into from

Conversation

pcmoritz
Copy link
Contributor

@pcmoritz pcmoritz commented Sep 27, 2016

The task queue is replaced by a task log, which is an append only datastructure (a hash table from task_iids to a list of scheduling updates). It reflects a message bus api and is used for pub/sub and also to store the task log in redis to make the pub/sub reliable when clients are disconnected.

There is a new datastructure, scheduled_task which is the information that is sent around to the schedulers via pub sub and can be reconstructed from the task log.

/* The scheduling_state can be used as a flag when we are listening for an event,
* for example TASK_WAITING | TASK_SCHEDULED. */
enum scheduling_state {
TASK_WAITING = 0,
Copy link
Collaborator

@stephanie-wang stephanie-wang Sep 27, 2016

Choose a reason for hiding this comment

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

I think that if we're going to use bitmasks to filter these, we will probably need to start the enum at 1, not 0 (otherwise, we can't differentiate between an event that we didn't register for and TASK_WAITING).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I am changing this

TASK_DONE = 4
};

typedef struct scheduled_task_impl scheduled_task;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe scheduled_task -> assigned_task?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm renaming it to task_instance now, which is what it really is! We are already using the assign terminology to refer to assigning tasks to workers, so it is not great in the context I think.

#include "task.h"

/* Callback for subscribing to the task log. */
typedef void (*task_log_callback)(scheduled_task* task, void *userdata);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add the state of the task to the arguments of the callback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is in scheduled_task

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.

2 participants