This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project iwhd.git.
The branch, master has been updated
via 3a8570e05e1e906435c97f456150ec8683b35c91 (commit)
via e8e1e84c9af5996630f65b9bf0b9dbe1108092e9 (commit)
from 8e27a1a5623d6fe674ebcaefa5ae43cae372873f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
http://repo.or.cz/w/iwhd.git/commit/3a8570e05e1e906435c97f456150ec8683b35c91
commit 3a8570e05e1e906435c97f456150ec8683b35c91
Author: Jeff Darcy <jdarcy(a)redhat.com>
Date: Tue Oct 19 21:10:49 2010 -0400
Fix two pipe-initialization races that occur in replication.
Turns out there were two race conditions, either of which could lead to
a replication task hanging. In the first, the consumer thread could
potentially run before the pipe_shared structure was re-initialized in
the producer. In the second, the producer could post the first chunk
before the consumer initialized its pipe_private, so the sequence
numbers would be off. That's kind of ironic, since the sequence number
isn't strictly necessary and is only there to guard against a whole
different class of synchronization problems (for which purpose I think
it should remain). Here's a brief summary.
* replica.c: moved re-initialization of pipe_shared to be before thread
start (race #1)
* mpipe.c: changed initialization of pp->sequence in pipe_init_private
(race #2)
* iwh.h: added PID to DPRINTF to distinguish parent/child output
* backend.c: fixed bytes==0 behavior in fs_put_child (unrelated)
* rest.c: fixed pipe_prod_wait_init check in proxy_put_data (unrelated)
diff --git a/backend.c b/backend.c
index a2972d1..502ce14 100644
--- a/backend.c
+++ b/backend.c
@@ -1189,24 +1189,24 @@ fs_put_child (void * ctx)
pipe_cons_siginit(ps, 0);
while (pipe_cons_wait(pp)) {
- offset = 0;
- do {
+ for (offset = 0; offset < ps->data_len; offset += bytes) {
bytes = write(fd,
ps->data_ptr+offset,ps->data_len-offset);
if (bytes <= 0) {
if (bytes < 0) {
error (0, errno, "%s: write failed",
file);
+ pipe_cons_signal(pp, errno);
}
- pipe_cons_signal(pp, errno);
- goto done;
+ else {
+ pipe_cons_signal(pp, ENOSPC);
+ }
+ break;
}
- offset += bytes;
- } while (offset < ps->data_len);
+ }
pipe_cons_signal(pp, 0);
}
-done:
close(fd);
DPRINTF("%s returningn",__func__);
diff --git a/iwh.h b/iwh.h
index a97322d..d6475de 100644
--- a/iwh.h
+++ b/iwh.h
@@ -28,11 +28,11 @@ GLOBAL(const char *, db_host, "localhost");
GLOBAL(unsigned short, db_port, 0);
GLOBAL(const char *, me, "here");
-#define DPRINTF(fmt,args...) do { - if (verbose)
{ - printf(fmt,##args); - fflush(stdout); - } +#define DPRINTF(fmt,args...) do
{ + if (verbose) { + printf("%d "
fmt,getpid(),##args); + fflush(stdout); + } } while (0)
#ifndef __attribute__
diff --git a/mpipe.c b/mpipe.c
index 0570316..46b3ad2 100644
--- a/mpipe.c
+++ b/mpipe.c
@@ -51,7 +51,12 @@ pipe_init_private (pipe_shared *ps)
pp = malloc(sizeof(*pp));
if (pp) {
pp->shared = ps;
- pp->sequence = ps->sequence + 1;
+ /*
+ * The producer might already have posted #1, so we can't use
+ * ps->sequence+1. This precludes consumers joining
+ * mid-stream, but that was never a goal anyway.
+ */
+ pp->sequence = 1;
pp->offset = 0;
}
return pp;
@@ -63,6 +68,7 @@ pipe_cons_wait (pipe_private *pp)
pipe_shared *ps = pp->shared;
int rc;
+ pp->offset = 0;
pthread_mutex_lock(&ps->lock);
while (ps->sequence != pp->sequence) {
@@ -212,4 +218,5 @@ pipe_prod_finish (pipe_shared *ps)
ps->cons_total,ps->cons_done,ps->cons_error);
}
pthread_mutex_unlock(&ps->lock);
+ DPRINTF("producer finished with sequence %ldn",ps->sequence);
}
diff --git a/replica.c b/replica.c
index 6170a83..4376d0a 100644
--- a/replica.c
+++ b/replica.c
@@ -95,13 +95,6 @@ proxy_repl_cons (void *ctx)
pipe_private *pp;
void *rc;
- /*
- * Do a full initialization here, not just in the rest. It's
- * necessary in the oddball case where we're re-replicating as
- * a result of an attribute/policy change, and it's not harmful
- * in the normal case where we're actually storing a new file.
- */
- pipe_init_shared(&ms->pipe,ms,1);
pp = pipe_init_private(&ms->pipe);
if (!pp) {
pipe_cons_siginit(&ms->pipe,-1);
@@ -177,6 +170,7 @@ repl_worker (void *notused ATTRIBUTE_UNUSED)
repl_item *item;
pthread_t cons;
pthread_t prod;
+ my_state *ms;
for (;;) {
sem_wait(&queue_sema);
@@ -188,6 +182,14 @@ repl_worker (void *notused ATTRIBUTE_UNUSED)
}
pthread_mutex_unlock(&queue_lock);
+ /*
+ * Do a full initialization here, not just in the rest. It's
+ * necessary in the oddball case where we're re-replicating as
+ * a result of an attribute/policy change, and it's not harmful
+ * in the normal case where we're actually storing a new file.
+ */
+ ms = item->ms;
+ pipe_init_shared(&ms->pipe,ms,1);
switch (item->type) {
case REPL_PUT:
if (pipe(item->pipes) >= 0) {
diff --git a/rest.c b/rest.c
index d09e7c1..28bc401 100644
--- a/rest.c
+++ b/rest.c
@@ -463,7 +463,7 @@ proxy_put_data (void *cctx, struct MHD_Connection *conn, const char
*url,
* out peace.
*/
rc = pipe_prod_wait_init(&ms->pipe);
- if (rc < 0) {
+ if (rc != 0) {
DPRINTF("producer wait failedn");
resp = MHD_create_response_from_data(0,NULL,
MHD_NO,MHD_NO);
http://repo.or.cz/w/iwhd.git/commit/e8e1e84c9af5996630f65b9bf0b9dbe1108092e9
commit e8e1e84c9af5996630f65b9bf0b9dbe1108092e9
Author: Jeff Darcy <jdarcy(a)redhat.com>
Date: Tue Oct 19 20:59:33 2010 -0400
Undo accidental reversal of wait_for_repl return values.
diff --git a/t/replication b/t/replication
index 78911cc..e9a26d5 100644
--- a/t/replication
+++ b/t/replication
@@ -47,8 +47,8 @@ api=http://localhost:$port
wait_for_repl() {
local n_req=$(curl -d op=rep_status $api) || return 1
case $n_req in
- '0 requests') return 1 ;;
- *) return 0 ;;
+ '0 requests') return 0 ;;
+ *) return 1 ;;
esac
}
bkt=$api/rbucket
-----------------------------------------------------------------------
Summary of changes:
backend.c | 14 +++++++-------
iwh.h | 10 +++++-----
mpipe.c | 9 ++++++++-
replica.c | 16 +++++++++-------
rest.c | 2 +-
t/replication | 4 ++--
6 files changed, 32 insertions(+), 23 deletions(-)
repo.or.cz automatic notification. Contact project admin jim(a)meyering.net
if you want to unsubscribe, or site admin admin(a)repo.or.cz if you receive
no reply.
--
iwhd.git ("image warehouse daemon")