fix job merging

This commit is contained in:
Lennart Poettering 2010-01-21 00:51:37 +01:00
parent 9ea024f6b5
commit 1ffba6fe82
8 changed files with 371 additions and 88 deletions

1
.gitignore vendored
View File

@ -1,3 +1,4 @@
systemd
*.o
test-engine
test-job-type

View File

@ -3,7 +3,7 @@ LIBS=-lrt
COMMON=name.o util.o set.o hashmap.o strv.o job.o manager.o conf-parser.o load-fragment.o socket-util.o log.o
all: systemd test-engine
all: systemd test-engine test-job-type
systemd: main.o $(COMMON)
$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
@ -11,5 +11,8 @@ systemd: main.o $(COMMON)
test-engine: test-engine.o $(COMMON)
$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
test-job-type: test-job-type.o $(COMMON)
$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
clean:
rm -f *.o systemd test-engine

100
job.c
View File

@ -1,6 +1,7 @@
/*-*- Mode: C; c-basic-offset: 8 -*-*/
#include <assert.h>
#include <errno.h>
#include "macro.h"
#include "job.h"
@ -127,7 +128,7 @@ void job_dependency_delete(Job *subject, Job *object, bool *matters) {
job_dependency_free(l);
}
void job_dump(Job *j, FILE*f, const char *prefix) {
const char* job_type_to_string(JobType t) {
static const char* const job_type_table[_JOB_TYPE_MAX] = {
[JOB_START] = "start",
@ -139,6 +140,15 @@ void job_dump(Job *j, FILE*f, const char *prefix) {
[JOB_TRY_RESTART] = "try-restart",
};
if (t < 0 || t >= _JOB_TYPE_MAX)
return "n/a";
return job_type_table[t];
}
void job_dump(Job *j, FILE*f, const char *prefix) {
static const char* const job_state_table[_JOB_STATE_MAX] = {
[JOB_WAITING] = "waiting",
[JOB_RUNNING] = "running",
@ -153,7 +163,7 @@ void job_dump(Job *j, FILE*f, const char *prefix) {
"%s\tAction: %s → %s\n"
"%s\tState: %s\n",
prefix, j->id,
prefix, name_id(j->name), job_type_table[j->type],
prefix, name_id(j->name), job_type_to_string(j->type),
prefix, job_state_table[j->state]);
}
@ -168,3 +178,89 @@ bool job_is_anchor(Job *j) {
return false;
}
static bool types_match(JobType a, JobType b, JobType c, JobType d) {
return
(a == c && b == d) ||
(a == d && b == c);
}
int job_type_merge(JobType *a, JobType b) {
if (*a == b)
return 0;
/* Merging is associative! a merged with b merged with c is
* the same as a merged with c merged with b. */
/* Mergeability is transitive! if a can be merged with b and b
* with c then a also with c */
/* Also, if a merged with b cannot be merged with c, then
* either a or b cannot be merged with c either */
if (types_match(*a, b, JOB_START, JOB_VERIFY_STARTED))
*a = JOB_START;
else if (types_match(*a, b, JOB_START, JOB_RELOAD) ||
types_match(*a, b, JOB_START, JOB_RELOAD_OR_START) ||
types_match(*a, b, JOB_VERIFY_STARTED, JOB_RELOAD_OR_START) ||
types_match(*a, b, JOB_RELOAD, JOB_RELOAD_OR_START))
*a = JOB_RELOAD_OR_START;
else if (types_match(*a, b, JOB_START, JOB_RESTART) ||
types_match(*a, b, JOB_START, JOB_TRY_RESTART) ||
types_match(*a, b, JOB_VERIFY_STARTED, JOB_RESTART) ||
types_match(*a, b, JOB_RELOAD, JOB_RESTART) ||
types_match(*a, b, JOB_RELOAD_OR_START, JOB_RESTART) ||
types_match(*a, b, JOB_RELOAD_OR_START, JOB_TRY_RESTART) ||
types_match(*a, b, JOB_RESTART, JOB_TRY_RESTART))
*a = JOB_RESTART;
else if (types_match(*a, b, JOB_VERIFY_STARTED, JOB_RELOAD))
*a = JOB_RELOAD;
else if (types_match(*a, b, JOB_VERIFY_STARTED, JOB_TRY_RESTART) ||
types_match(*a, b, JOB_RELOAD, JOB_TRY_RESTART))
*a = JOB_TRY_RESTART;
else
return -EEXIST;
return 0;
}
bool job_type_mergeable(JobType a, JobType b) {
return job_type_merge(&a, b) >= 0;
}
bool job_type_is_superset(JobType a, JobType b) {
/* Checks whether operation a is a "superset" of b */
if (a == b)
return true;
switch (a) {
case JOB_START:
return b == JOB_VERIFY_STARTED;
case JOB_RELOAD:
return b == JOB_VERIFY_STARTED;
case JOB_RELOAD_OR_START:
return
b == JOB_RELOAD ||
b == JOB_START;
case JOB_RESTART:
return
b == JOB_START ||
b == JOB_VERIFY_STARTED ||
b == JOB_RELOAD ||
b == JOB_RELOAD_OR_START ||
b == JOB_TRY_RESTART;
case JOB_TRY_RESTART:
return
b == JOB_VERIFY_STARTED ||
b == JOB_RELOAD;
default:
return false;
}
}

5
job.h
View File

@ -90,4 +90,9 @@ bool job_is_anchor(Job *j);
int job_merge(Job *j, Job *other);
const char* job_type_to_string(JobType t);
int job_type_merge(JobType *a, JobType b);
bool job_type_mergeable(JobType a, JobType b);
bool job_type_is_superset(JobType a, JobType b);
#endif

259
manager.c
View File

@ -55,12 +55,24 @@ static void transaction_delete_job(Manager *m, Job *j) {
assert(m);
assert(j);
/* Deletes one job from the transaction */
manager_transaction_unlink_job(m, j);
if (!j->linked)
job_free(j);
}
static void transaction_delete_name(Manager *m, Name *n) {
Job *j;
/* Deletes all jobs associated with a certain name from the
* transaction */
while ((j = hashmap_get(m->transaction_jobs, n)))
transaction_delete_job(m, j);
}
static void transaction_abort(Manager *m) {
Job *j;
@ -81,6 +93,11 @@ static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsi
assert(m);
/* A recursive sweep through the graph that marks all names
* that matter to the anchor job, i.e. are directly or
* indirectly a dependency of the anchor job via paths that
* are fully marked as mattering. */
for (l = j ? j->subject_list : m->transaction_anchor; l; l = l->subject_next) {
/* This link does not matter */
@ -98,40 +115,6 @@ static void transaction_find_jobs_that_matter_to_anchor(Manager *m, Job *j, unsi
}
}
static bool types_match(JobType a, JobType b, JobType c, JobType d) {
return
(a == c && b == d) ||
(a == d && b == c);
}
static int types_merge(JobType *a, JobType b) {
if (*a == b)
return 0;
if (types_match(*a, b, JOB_START, JOB_VERIFY_STARTED))
*a = JOB_START;
else if (types_match(*a, b, JOB_START, JOB_RELOAD) ||
types_match(*a, b, JOB_START, JOB_RELOAD_OR_START) ||
types_match(*a, b, JOB_VERIFY_STARTED, JOB_RELOAD_OR_START) ||
types_match(*a, b, JOB_RELOAD, JOB_RELOAD_OR_START))
*a = JOB_RELOAD_OR_START;
else if (types_match(*a, b, JOB_START, JOB_RESTART) ||
types_match(*a, b, JOB_START, JOB_TRY_RESTART) ||
types_match(*a, b, JOB_VERIFY_STARTED, JOB_RESTART) ||
types_match(*a, b, JOB_RELOAD, JOB_RESTART) ||
types_match(*a, b, JOB_RELOAD_OR_START, JOB_RESTART) ||
types_match(*a, b, JOB_RELOAD_OR_START, JOB_TRY_RESTART) ||
types_match(*a, b, JOB_RESTART, JOB_TRY_RESTART))
*a = JOB_RESTART;
else if (types_match(*a, b, JOB_VERIFY_STARTED, JOB_RELOAD))
*a = JOB_RELOAD;
else if (types_match(*a, b, JOB_VERIFY_STARTED, JOB_TRY_RESTART) ||
types_match(*a, b, JOB_RELOAD, JOB_TRY_RESTART))
*a = JOB_TRY_RESTART;
return -EEXIST;
}
static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, JobType t) {
JobDependency *l, *last;
@ -140,6 +123,8 @@ static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, Job
assert(j->name == other->name);
assert(!j->linked);
/* Merges 'other' into 'j' and then deletes j. */
j->type = t;
j->state = JOB_WAITING;
@ -183,6 +168,44 @@ static void transaction_merge_and_delete_job(Manager *m, Job *j, Job *other, Job
transaction_delete_job(m, other);
}
static int delete_one_unmergable_job(Manager *m, Job *j) {
Job *k;
assert(j);
/* Tries to delete one item in the linked list
* j->transaction_next->transaction_next->... that conflicts
* whith another one, in an attempt to make an inconsistent
* transaction work. */
/* We rely here on the fact that if a merged with b does not
* merge with c, either a or b merge with c neither */
for (; j; j = j->transaction_next)
for (k = j->transaction_next; k; k = k->transaction_next) {
Job *d;
/* Is this one mergeable? Then skip it */
if (job_type_mergeable(j->type, k->type))
continue;
/* Ok, we found two that conflict, let's see if we can
* drop one of them */
if (!j->matters_to_anchor)
d = j;
else if (!k->matters_to_anchor)
d = k;
else
return -ENOEXEC;
/* Ok, we can drop one, so let's do so. */
log_debug("Try to fix job merging by deleting job %s", name_id(d->name));
transaction_delete_job(m, d);
return 0;
}
return -EINVAL;
}
static int transaction_merge_jobs(Manager *m) {
Job *j;
void *state;
@ -190,13 +213,39 @@ static int transaction_merge_jobs(Manager *m) {
assert(m);
/* First step, check whether any of the jobs for one specific
* task conflict. If so, try to drop one of them. */
HASHMAP_FOREACH(j, m->transaction_jobs, state) {
JobType t;
Job *k;
t = j->type;
for (k = j->transaction_next; k; k = k->transaction_next) {
if ((r = job_type_merge(&t, k->type)) >= 0)
continue;
/* OK, we could not merge all jobs for this
* action. Let's see if we can get rid of one
* of them */
if ((r = delete_one_unmergable_job(m, j)) >= 0)
/* Ok, we managed to drop one, now
* let's ask our callers to call us
* again after garbage collecting */
return -EAGAIN;
/* We couldn't merge anything. Failure */
return r;
}
}
/* Second step, merge the jobs. */
HASHMAP_FOREACH(j, m->transaction_jobs, state) {
JobType t = j->type;
Job *k;
for (k = j->transaction_next; k; k = k->transaction_next)
if ((r = types_merge(&t, k->type)) < 0)
return r;
assert_se(job_type_merge(&t, k->type) == 0);
while ((k = j->transaction_next)) {
if (j->linked) {
@ -213,6 +262,20 @@ static int transaction_merge_jobs(Manager *m) {
return 0;
}
static bool name_matters_to_anchor(Name *n, Job *j) {
assert(n);
assert(!j->transaction_prev);
/* Checks whether at least one of the jobs for this name
* matters to the anchor. */
for (; j; j = j->transaction_next)
if (j->matters_to_anchor)
return true;
return false;
}
static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned generation) {
void *state;
Name *n;
@ -220,19 +283,30 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned
assert(m);
assert(j);
assert(!j->transaction_prev);
/* Does a recursive sweep through the ordering graph, looking
* for a cycle. If we find cycle we try to break it. */
/* Did we find a cycle? */
if (j->marker && j->generation == generation) {
Job *k;
/* So, we already have been here. We have a
* cycle. Let's try to break it. We go backwards in our
* path and try to find a suitable job to remove. */
* cycle. Let's try to break it. We go backwards in
* our path and try to find a suitable job to
* remove. We use the marker to find our way back,
* since smart how we are we stored our way back in
* there. */
for (k = from; k; k = (k->generation == generation ? k->marker : NULL)) {
if (!k->matters_to_anchor) {
if (!k->linked &&
!name_matters_to_anchor(k->name, k)) {
/* Ok, we can drop this one, so let's
* do so. */
log_debug("Breaking order cycle by deleting job %s", name_id(k->name));
transaction_delete_job(m, k);
transaction_delete_name(m, k->name);
return -EAGAIN;
}
@ -242,19 +316,25 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned
break;
}
return -ELOOP;
return -ENOEXEC;
}
/* Make the marker point to where we come from, so that we can
* find our way backwards if we want to break a cycle */
j->marker = from;
j->generation = generation;
/* We assume that the the dependencies are both-ways, and
/* We assume that the the dependencies are bidirectional, and
* hence can ignore NAME_AFTER */
SET_FOREACH(n, j->name->meta.dependencies[NAME_BEFORE], state) {
Job *o;
/* Is there a job for this name? */
if (!(o = hashmap_get(m->transaction_jobs, n)))
/* Ok, there is no job for this in the
* transaction, but maybe there is already one
* running? */
if (!(o = n->meta.job))
continue;
@ -266,36 +346,19 @@ static int transaction_verify_order_one(Manager *m, Job *j, Job *from, unsigned
}
static int transaction_verify_order(Manager *m, unsigned *generation) {
bool again;
Job *j;
int r;
void *state;
assert(m);
assert(generation);
do {
Job *j;
int r;
void *state;
/* Check if the ordering graph is cyclic. If it is, try to fix
* that up by dropping one of the jobs. */
again = false;
HASHMAP_FOREACH(j, m->transaction_jobs, state) {
/* Assume merged */
assert(!j->transaction_next);
assert(!j->transaction_prev);
if ((r = transaction_verify_order_one(m, j, NULL, (*generation)++)) < 0) {
/* There was a cycleq, but it was fixed,
* we need to restart our algorithm */
if (r == -EAGAIN) {
again = true;
break;
}
return r;
}
}
} while (again);
HASHMAP_FOREACH(j, m->transaction_jobs, state)
if ((r = transaction_verify_order_one(m, j, NULL, (*generation)++)) < 0)
return r;
return 0;
}
@ -305,6 +368,8 @@ static void transaction_collect_garbage(Manager *m) {
assert(m);
/* Drop jobs that are not required by any other job */
do {
void *state;
Job *j;
@ -335,7 +400,9 @@ static int transaction_is_destructive(Manager *m, JobMode mode) {
* existing jobs would be replaced */
HASHMAP_FOREACH(j, m->transaction_jobs, state)
if (j->name->meta.job && j->name->meta.job != j)
if (j->name->meta.job &&
j->name->meta.job != j &&
!job_type_is_superset(j->type, j->name->meta.job->type))
return -EEXIST;
return 0;
@ -346,6 +413,8 @@ static int transaction_apply(Manager *m, JobMode mode) {
Job *j;
int r;
/* Moves the transaction jobs to the set of active jobs */
HASHMAP_FOREACH(j, m->transaction_jobs, state) {
if (j->linked)
continue;
@ -376,6 +445,8 @@ static int transaction_apply(Manager *m, JobMode mode) {
job_dependency_free(j->object_list);
}
m->transaction_anchor = NULL;
return 0;
rollback:
@ -403,23 +474,47 @@ static int transaction_activate(Manager *m, JobMode mode) {
/* First step: figure out which jobs matter */
transaction_find_jobs_that_matter_to_anchor(m, NULL, generation++);
/* Second step: let's merge entries we can merge */
if ((r = transaction_merge_jobs(m)) < 0)
goto rollback;
for (;;) {
/* Second step: Let's remove unneeded jobs that might
* be lurking. */
transaction_collect_garbage(m);
/* Third step: verify order makes sense */
if ((r = transaction_verify_order(m, &generation)) < 0)
goto rollback;
/* Third step: verify order makes sense and correct
* cycles if necessary and possible */
if ((r = transaction_verify_order(m, &generation)) >= 0)
break;
/* Third step: do garbage colletion */
transaction_collect_garbage(m);
if (r != -EAGAIN)
goto rollback;
/* Fourth step: check whether we can actually apply this */
/* Let's see if the resulting transaction ordering
* graph is still cyclic... */
}
for (;;) {
/* Fourth step: let's drop unmergable entries if
* necessary and possible, merge entries we can
* merge */
if ((r = transaction_merge_jobs(m)) >= 0)
break;
if (r != -EAGAIN)
goto rollback;
/* Fifth step: an entry got dropped, let's garbage
* collect its dependencies. */
transaction_collect_garbage(m);
/* Let's see if the resulting transaction still has
* unmergable entries ... */
}
/* Sith step: check whether we can actually apply this */
if (mode == JOB_FAIL)
if ((r = transaction_is_destructive(m, mode)) < 0)
goto rollback;
/* Fifth step: apply changes */
/* Seventh step: apply changes */
if ((r = transaction_apply(m, mode)) < 0)
goto rollback;
@ -664,10 +759,10 @@ int manager_load_name(Manager *m, const char *name, Name **_ret) {
return -ENOMEM;
}
if (set_put(ret->meta.names, n) < 0) {
if ((r = set_put(ret->meta.names, n)) < 0) {
name_free(ret);
free(n);
return -ENOMEM;
return r;
}
if ((r = name_link(ret)) < 0) {

View File

@ -9,7 +9,7 @@
int main(int argc, char *argv[]) {
Manager *m = NULL;
Name *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e;
Name *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *g = NULL;
Job *j;
assert_se(chdir("test2") == 0);
@ -33,13 +33,28 @@ int main(int argc, char *argv[]) {
manager_dump_names(m, stdout, "\t");
printf("Test2: (Cyclic Order, Unfixable)\n");
assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, false, &j) == -ELOOP);
assert_se(manager_add_job(m, JOB_START, d, JOB_REPLACE, false, &j) == -ENOEXEC);
manager_dump_jobs(m, stdout, "\t");
printf("Test3: (Cyclic Order, Fixable)\n");
printf("Test3: (Cyclic Order, Fixable, Garbage Collector)\n");
assert_se(manager_add_job(m, JOB_START, e, JOB_REPLACE, false, &j) == 0);
manager_dump_jobs(m, stdout, "\t");
printf("Test4: (Identical transaction)\n");
assert_se(manager_add_job(m, JOB_START, e, JOB_FAIL, false, &j) == 0);
manager_dump_jobs(m, stdout, "\t");
printf("Loaded3:\n");
assert_se(manager_load_name(m, "g.service", &g) == 0);
manager_dump_names(m, stdout, "\t");
printf("Test5: (Colliding transaction, fail)\n");
assert_se(manager_add_job(m, JOB_START, g, JOB_FAIL, false, &j) == -EEXIST);
printf("Test6: (Colliding transaction, replace)\n");
assert_se(manager_add_job(m, JOB_START, g, JOB_REPLACE, false, &j) == 0);
manager_dump_jobs(m, stdout, "\t");
manager_free(m);
return 0;

65
test-job-type.c Normal file
View File

@ -0,0 +1,65 @@
/*-*- Mode: C; c-basic-offset: 8 -*-*/
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include "job.h"
int main(int argc, char*argv[]) {
JobType a, b, c, d, e, f, g;
for (a = 0; a < _JOB_TYPE_MAX; a++)
for (b = 0; b < _JOB_TYPE_MAX; b++) {
if (!job_type_mergeable(a, b))
printf("Not mergeable: %s + %s\n", job_type_to_string(a), job_type_to_string(b));
for (c = 0; c < _JOB_TYPE_MAX; c++) {
/* Verify transitivity of mergeability
* of job types */
assert(!job_type_mergeable(a, b) ||
!job_type_mergeable(b, c) ||
job_type_mergeable(a, c));
d = a;
if (job_type_merge(&d, b) >= 0) {
printf("%s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(d));
/* Verify that merged entries can be
* merged with the same entries they
* can be merged with seperately */
assert(!job_type_mergeable(a, c) || job_type_mergeable(d, c));
assert(!job_type_mergeable(b, c) || job_type_mergeable(d, c));
/* Verify that if a merged
* with b is not mergable with
* c then either a or b is not
* mergeable with c either. */
assert(job_type_mergeable(d, c) || !job_type_mergeable(a, c) || !job_type_mergeable(b, c));
e = b;
if (job_type_merge(&e, c) >= 0) {
/* Verify associativity */
f = d;
assert(job_type_merge(&f, c) == 0);
g = e;
assert(job_type_merge(&g, a) == 0);
assert(f == g);
printf("%s + %s + %s = %s\n", job_type_to_string(a), job_type_to_string(b), job_type_to_string(c), job_type_to_string(d));
}
}
}
}
return 0;
}

3
test2/g.service Normal file
View File

@ -0,0 +1,3 @@
[Meta]
Description=G
Conflicts=e.service