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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
