Revert "Release RowExclusiveLock on pg_dist_transaction as soon as remote xacts are recovered"

[onurctirtir]

This reverts commit 684b4c6.

--

In #7863, we attempted to fix #7875 but it doesn't seem like a proper way of doing that, hence reverting.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.46%. Comparing base (c55bc8c) to head (273932d).
Report is 16 commits behind head on release-13.0.

Additional details and impacted files

@@               Coverage Diff                @@
##           release-13.0    #7877      +/-   ##
================================================
- Coverage         89.48%   89.46%   -0.02%     
================================================
  Files               276      276              
  Lines             60063    59892     -171     
  Branches           7524     7510      -14     
================================================
- Hits              53747    53584     -163     
+ Misses             4166     4165       -1     
+ Partials           2150     2143       -7     

[marcoslot]

Thanks. Not sure what can happen after the lock gets released, but since DDL commands normally don't anticipate having uncommitted tuples you might get into nasty situations (e.g. table rewrite during ALTER TABLE incorrectly treats tuple as committed/aborted).

Probably there needs to be an "update extension lock" somewhere that freezes the maintenance daemon to avoid deadlocking with it.

I believe that was the goal of LockCitusExtension, but not sure that the lock happens on the other end. Probably a utility hook for ALTER EXTENSION should do that.

[onurctirtir]

Thanks. Not sure what can happen after the lock gets released, but since DDL commands normally don't anticipate having uncommitted tuples you might get into nasty situations (e.g. table rewrite during ALTER TABLE incorrectly treats tuple as committed/aborted).

Probably there needs to be an "update extension lock" somewhere that freezes the maintenance daemon to avoid deadlocking with it.

I believe that was the goal of LockCitusExtension, but not sure that the lock happens on the other end. Probably a utility hook for ALTER EXTENSION should do that.

Thanks Marco for the heads-up, once again!

I believe that was the goal of LockCitusExtension, but not sure that the lock happens on the other end. Probably a utility hook for ALTER EXTENSION should do that.

And it's very good to know that because @codeforall also suggested a similar idea to fix this but I wasn't aware of the fact that this was the goal of LockCitusExtension() already.

We'll further investigate this, thanks!