Re: PATCH: pgbench - merging transaction logs - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: PATCH: pgbench - merging transaction logs
Date
Msg-id alpine.DEB.2.10.1503160746380.4015@sto
Whole thread Raw
In response to Re: PATCH: pgbench - merging transaction logs  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
List 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.



pgsql-hackers by date:

Previous
From: Kyotaro HORIGUCHI
Date:
Subject: Re: Reduce pinning in btree indexes
Next
From: Amit Kapila
Date:
Subject: Re: Parallel Seq Scan