Christopher Browne cbbrowne at afilias.info
Mon Nov 12 08:33:13 PST 2012
On Mon, Nov 12, 2012 at 10:39 AM, Dave Page <dpage at pgadmin.org> wrote:
> On Mon, Nov 12, 2012 at 3:28 PM, Devrim GÜNDÜZ <devrim at gunduz.org> wrote:
>>
>> Hi,
>>
>> On Mon, 2012-11-12 at 15:11 +0000, Luiz K.Matsumura wrote:
>>> Change get_pid to return the smallest pid
>>>
>>> get_pid changed to return the smallest pid of processes that match
>>> the regex, probably the main proccess
>>
>> Uh? You changed a behavior based on something "probably"?
>
> I missed any discussion on this as well as the commit itself, so I may
> be missing some context... but assuming pids are sequential seems
> problematic, particularly on Windows where they may not actually be
> assigned sequentially. On *nix of course, the counter can wrap...

On AIX, which is a UNIX(tm), PIDs are not assigned at all
sequentially.  (At least, not on versions I've touched lately.)  And
in other cases, "typically the smallest PID" does indeed have an
unfortunate failure case.

I'll note that Steve Singer and I both noted this issue, and commented
on it (see https://github.com/lkmatsumura/slony1-engine/commit/f25c29f91d61ca0f48af70a6492af101d48e079a),
which led to a later patch
(https://github.com/lkmatsumura/slony1-engine/commit/95e67acb641c9620d68f2f01307bf3129a593d7c)
which uses the PID file generated by Slony.

I'm very pleased that y'all are noticing the issues; in fact, there's
nothing to see here, as the fixed-up result did get drawn in.

I suppose there is merit to debating whether we should:

a) Merge patches in directly, which will tend to lead to situations
like this where people can notice, and get concerned about, interim
states where the patch wasn't quite right yet, or

b) Cherry pick a merged form of the patch, thereby losing the shape of
those interim patches, but losing the entertainment of threads like
this one :-).

At any rate, thanks for keeping your eyes peeled!  'Tis a good thing to do!


More information about the Slony1-hackers mailing list