Re: PATCH: Add 'pid' column to pg_replication_slots
| От | Craig Ringer |
|---|---|
| Тема | Re: PATCH: Add 'pid' column to pg_replication_slots |
| Дата | |
| Msg-id | CAMsr+YG03fakzadtLDH0ybQG3J6R+-bU9mkZ3zLmHjokk79h5w@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: PATCH: Add 'pid' column to pg_replication_slots (Andres Freund <andres@anarazel.de>) |
| Ответы |
Re: PATCH: Add 'pid' column to pg_replication_slots
Re: PATCH: Add 'pid' column to pg_replication_slots |
| Список | pgsql-hackers |
On 21 April 2015 at 15:19, Andres Freund <andres@anarazel.de> wrote:
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.
Isn't that the point? We're acquiring the slot there, per the comment:
"Find a previously created slot and mark it as used by this backend."
> namecpy(&plugin, &slot->data.plugin);
>
> - active = slot->active;
> + active_pid = slot->active_pid != 0;
That doesn't look right.
No, that's certainly not right. I also could've sworn I sorted it out, but that must've been another site, because sure enough it's still there.
I don't really like the 'pid' field for pg_replication_slots. About
naming it 'active_in' or such?
It was originally named active_pid, but changed based on feedback from others that 'pid' would be consistent with pg_stat_activity and pg_replication_slots. I have no strong opinion on the name, though I'd prefer it reflect that the field does in fact represent a process ID.
В списке pgsql-hackers по дате отправления: