On 2015-04-07 18:41:59 +0800, Craig Ringer wrote: > @@ -331,8 +331,8 @@ ReplicationSlotAcquire(const char *name) > volatile ReplicationSlot *vslot = s; > > SpinLockAcquire(&s->mutex); > - active = vslot->active; > - vslot->active = true; > + active = vslot->active_pid != 0; > + vslot->active_pid = MyProcPid; > SpinLockRelease(&s->mutex); > slot = s; > break;
Uh. You're overwriting the existing pid here. Not good if the slot is currently in use. > namecpy(&plugin, &slot->data.plugin); > > - active = slot->active; > + active_pid = slot->active_pid != 0; That doesn't look right. > --- a/src/include/replication/slot.h > +++ b/src/include/replication/slot.h > @@ -84,13 +84,15 @@ typedef struct ReplicationSlot > /* is this slot defined */ > bool in_use; > > - /* is somebody streaming out changes for this slot */ > - bool active; > + /* field 'active' removed in 9.5; see 'active_pid' instead */ > > /* any outstanding modifications? */ > bool just_dirtied; > bool dirty; > > + /* Who is streaming out changes for this slot? 0 for nobody */ > + pid_t active_pid; > + That's a horrible idea. That way we end up with dozens of indirections over time. I don't really like the 'pid' field for pg_replication_slots. About naming it 'active_in' or such? Other than these I plan to push this soon. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
