Hello Marina,
Hello, Fabien!
A few comments about the submitted patches.
Thank you very much for them!
I agree that improving the error handling ability of pgbench is a good thing, although I'm not sure about the implications...
Could you tell a little bit more exactly.. What implications are you worried about?
About the "retry" discussion: I agree that retry is the relevant option from an application point of view.
I'm glad to hear it!
ISTM that the retry implementation should be implemented somehow in the automaton, restarting the same script for the beginning.
If there are several transactions in this script - don't you think that we should restart only the failed transaction?..
As pointed out in the discussion, the same values/commands should be executed, which suggests that random generated values should be the same on the retry runs, so that for a simple script the same operations are attempted. This means that the random generator state must be kept & reinstated for a client on retries. Currently the random state is in the thread, which is not convenient for this purpose, so it should be moved in the client so that it can be saved at transaction start and reinstated on retries.
I think about it in the same way =)
The number of retries and maybe failures should be counted, maybe with some adjustable maximum, as suggested.
If we fix the maximum number of attempts the maximum number of failures for one script execution will be bounded above (number_of_transactions_in_script * maximum_number_of_attempts). Do you think we should make the option in program to limit this number much more?
About 0001: In accumStats, just use one level if, the two levels bring nothing.
Thanks, I agree =[
In doLog, added columns should be at the end of the format.
I have inserted it earlier because these columns are not optional. Do you think they should be optional?
The number of column MUST NOT change when different issues arise, so that it works well with cut/... unix commands, so inserting a sentence such as "serialization and deadlock failures" is a bad idea.
Thanks, I agree again.
threadRun: the point of the progress format is to fit on one not too wide line on a terminal and to allow some simple automatic processing. Adding a verbose sentence in the middle of it is not the way to go.
I was thinking about it.. Thanks, I'll try to make it shorter.
About tests: I do not understand why test 003 includes 2 transactions. It would seem more logical to have two scripts.
Ok!
About 0003: I'm not sure that there should be an new option to report failures, the information when relevant should be integrated in a clean format into the existing reports... Maybe the "per command latency" report/option should be renamed if it becomes more general.
I have tried do not change other parts of program as much as possible. But if you think that it will be more useful to change the option I'll do it.
About 0004: The documentation must not be in a separate patch, but in the same patch as their corresponding code.
Ok! -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
