On Thu, Aug 22, 2019 at 07:45:09PM +0530, Asim R P wrote:
> Fault injection was discussed a few months ago at PGCon in Ottawa. At
> least a few folks showed interest and so I would like to present what we
> have been using in Greenplum.
>
> The attached patch set contains the fault injector framework ported to
> PostgreSQL master. It provides ability to define points of interest in
> backend code and then inject faults at those points from SQL. Also
> included is an isolation test to simulate a speculative insert conflict
> scenario that was found to be rather cumbersome to implement using advisory
> locks, see [1]. The alternative isolation spec using fault injectors seems
> much simpler to understand.
You may want to double-check whitespaces in your patch set, and 0002
does not apply because of conflicts in isolationtester.h (my fault!).
0002 is an independent feature, so I would keep it out of the fault
framework for integration. There has been a argument from Alvaro
more convincing than mine about the use of a separate keyword, hence
removing a dependency with steps:
https://www.postgresql.org/message-id/20190823153825.GA11405@alvherre.pgsql
It would be good also to have a test case which exercises it, without
the need of the fault framework or its dedicated schedule.
Patches 0003, 0004 and 0005 could just be grouped together, they deal
about the same thing.
My first impressions about this patch is that it is very intrusive.
Could you explain the purpose of am_faultinjector? That's a specific
connection string parameter which can be used similarly to replication
for WAL senders? Couldn't there be an equivalent with a SUSET GUC?
It may be interesting to see which parts of this framework could be
moved into an extension loaded with shared_preload_libraries, one
thing being the shared memory initialization part. At the end it
would be interesting to not have a dependency with a compile-time
flag.
Things like exec_fault_injector_command() need to be more documented.
It is hard to guess what it is being used for.
--
Michael