Re: PATCH: pgbench - merging transaction logs

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: pgbench - merging transaction logs
Date: 2015-03-16 07:02:46
Message-ID: alpine.DEB.2.10.1503160746380.4015@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


>> I agree that inter-process stuff should be avoided. This is not what
>> I had in mind. I was thinking of "fprintf" on the same file handler
>> by different threads.
>
> That still involves some sort of 'implicit' locking, no?

I fully agree that it would need locking, wether it is implicit if fprintf
is thread safe, or explicit if not.

> And as I mentioned, I'm not sure fprintf() is thread-safe on all the
> platforms we support.

I do not know, but the question is relevant ! The answer must be thought
for. Adding a MUTEX to be on the safe side may not be too big an issue.

>>>> Another issue raised by your patch is that the log format may be
>>>> improved, say by providing a one-field timestamp at the beginning
>>>> of the line.
>>>
>>> I don't see how this is relevant to this patch?
>>
>> For the simple log format, all the parsing needed would be for the
>> timestamp, and the remainder would just be text to pass along, no
>> need to %d %f... whatever.
>
> Oh, ok. Well, that's true, but I don't think that significantly changes
> the overall complexity.

For your current implementation, it would remove plenty of variables,
conditions about options, just one scan one print would remain, and no
need to update the format later, so it would simplify the code greatly for
the "simple" log.

>> I would suggest this that I would support: implement this feature the
>> simple way (aka fprintf, maybe a mutex) when compiled with threads, and
>> generate an error "feature not available with process-based thread
>> emulation" when compiled with processes. This way we avoid a, lot of
>> heavy code to maintain in the future, and you still get the feature
>> within pgbench. There are already some things which are not the same
>> with thread emulation because it would have been tiring to implement for
>> it for very little benefit.
>
> I really dislike the features supported only in some cases (although
> most deployments probably support proper threading these days).

I agree on the general principle, but not in this particular case, because
for me thread emulation is more or less obsolete, useless and unused.

My view is that you're code is on the too-heavy side, and adds a burden
for later maintenance for rather a should-be-simple feature, really just
so it works for "thread emulation" that probably nought people use or
really need. So I'm not enthousiastic at all to get that in pgbench as is.

If the code is much simpler, these reservations would be dropped, it would
be a small and useful feature for a small code and maintenance impact. And
I do not see the point in using the heavy stuff in my view obsolete stuff.

--
Fabien.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2015-03-16 07:14:00 Re: Parallel Seq Scan
Previous Message Kyotaro HORIGUCHI 2015-03-16 06:56:24 Re: Reduce pinning in btree indexes