Re: POC: Extension for adding distributed tracing - pg_tracing

Hi,

On 2024-02-09 19:46:58 +0300, Nikita Malakhov wrote:
> I agree with Heikki on most topics and especially the one he recommended
> to publish your extension on GitHub, this tool is very promising for highly
> loaded
> systems, you will get a lot of feedback very soon.
> 
> I'm curious about SpinLock - it is recommended for very short operations,
> taking up to several instructions, and docs say that for longer ones it
> will be
> too expensive, and recommends using LWLock. Why have you chosen SpinLock?
> Does it have some benefits here?

Indeed - e.g. end_tracing() looks to hold the spinlock for far too long for
spinlocks to be appropriate. Use an lwlock.

Random stuff I noticed while skimming:
- pg_tracing.c should include postgres.h as the first thing

- afaict none of the use of volatile is required, spinlocks have been barriers
  for a long time now

- you acquire the spinlock for single increments of n_writers, perhaps that
  could become an atomic, to reduce contention?

- I don't think it's a good idea to do memory allocations in the middle of a
  PG_CATCH. If the error was due to out-of-memory, you'll throw another error.


Greetings,

Andres Freund