Re: [HACKERS] Early locking option to parallel backup

Lucas, Robert, all,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Mon, Nov 6, 2017 at 4:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wonder if we couldn't somehow repurpose the work that was done for
parallel workers' locks. Lots of security-type issues to be handled
if we're to open that up to clients, but maybe it's solvable. For
instance, maybe only allowing it to clients sharing the same snapshot
would help.

Interesting idea. There's a bunch of holes that would need to be
patched there; for instance, you can't have one session running DDL
while somebody else has AccessShareLock. Parallel query relies on the
parallel-mode restrictions to prevent that kind of thing from
happening, but it would be strange (and likely somewhat broken) to try
to enforce those here. It would be strange and probably bad if LOCK
TABLE a; LOCK TABLE b in one session and LOCK TABLE b; LOCK TABLE a in
another session failed to deadlock. In short, there's a big
difference between a single session using multiple processes and
multiple closely coordinated sessions.

Parallel pg_dump is based on synchronized transactions though and we
have a bunch of checks in ImportSnapshot() because a pg_dump parallel
worker also can't really be quite the same as a normal backend. Perhaps
we could add on more restrictions in ImportSnapshot() to match the
restrictions for parallel-mode workers? If we think there's other users
of SET TRANSACTION SNAPSHOT then we might need to extend that command
for this case, but that seems relatively straight-forward. I don't know
how reasonable the idea of taking a normally-started backend and making
it close enough to a parallel worker when a SET TRANSACTION SNAPSHOT
PARALLEL (or whatever) happens to allow it to skip the lock fairness is
though.

Also, even if you did it, you still need a lot of PROCLOCKs. Workers
don't need to take all locks up front because they can be assured of
getting them later, but they've still got to lock the objects they
actually want to access. Group locking aims to prevent deadlocks
between cooperating processes; it is not a license to skip locking
altogether.

This wouldn't be any different from what's happening today in pg_dump
though, so I'm not sure why this would be an issue? The proposed patch
locks everything in every worker, which is quite different from the main
process locking everything and then the individual workers locking the
objects they're working on.

* Lucas B (lucas75@gmail.com) wrote:

Em 05/11/2017 21:09, Andres Freund escreveu:

Well, the current approach afaics requires #relations * 2 locks, whereas
acquiring them in every worker would scale that with the number of
workers.

Yes, that is why I proposed as an option. As an option will not affect
anyone that does not want to use it.

That's not actually correct- additional options add maintenance overhead
for hackers and for users to have to deal with and there's at least a
clear idea on how we could make this "just work" without having to add
an additional option. Based on that and the discussion thus far, it
seems like the next step is to try and work through a way to change
things to allow workers to skip the lock fairness and that the current
patch isn't going to be accepted.

It seems natural to think several connections in a synchronized snapshot as
the same connection. Then it may be reasonable to grant a shared lock out of
turn if any connection of the same shared snapshot already have a granted
lock for the same relation. Last year Tom mentioned that there is already
queue-jumping logic of that sort in the lock manager for other purposes.
Although seems conceptually simple, I suspect the implementation is not.

The implementation is almost certainly not simple, but that doesn't mean
we should go in another direction and require a great deal more locks or
add an option to do so.

I'm going to move this patch into 'Waiting for Author' since it's
clearly gotten a good bit of review and discussion already. If there's
no interest in further exploring the approach of changing the locking
logic then we should probably update the patch to RWF.

This seems like a good project to go on the TODO list though, if you
aren't going to pursue it, to further explore how to let parallel
pg_dump processes jump the lock queue, with a reference back to this
thread.

Thanks!

Stephen