On Wed, Jan 27, 2021 at 02:41:32PM +0100, Jehan-Guillaume de Rorthais wrote: > > On Wed, 27 Jan 2021 11:25:11 +0100 > Denis Laxalde <denis.laxa...@dalibo.com> wrote: > > > Andres Freund a écrit : > > > > I wonder if we could > > > > > > a) set default_transaction_read_only to true, and explicitly change it > > > in the sessions that need that. > > According to Denis' tests discussed off-list, it works fine in regard with the > powa bgworker, albeit some complaints in logs. However, I wonder how fragile > it > could be as bgworker could easily manipulate either the GUC or "BEGIN READ > WRITE". I realize this is really uncommon practices, but bgworker code from > third parties might be surprising.
Given that having any writes happening at the wrong moment on the old cluster can end up corrupting the new cluster, and that the corruption might not be immediately visible we should try to put as many safeguards as possible. so +1 for the default_transaction_read_only as done in Denis' patch at minimum, but not only. AFAICT it should be easy to prevent all background worker from being launched by adding a check on IsBinaryUpgrade at the beginning of bgworker_should_start_now(). It won't prevent modules from being loaded, so this approach should be problematic. > > > b) when in binary upgrade mode / -b, error out on all wal writes in > > > sessions that don't explicitly set a session-level GUC to allow > > > writes. > > It feels safer because more specific to the subject. But I wonder if the > benefice worth adding some (limited?) complexity and a small/quick conditional > block in a very hot code path for a very limited use case. I don't think that it would add that much complexity or overhead as there's already all the infrastructure to prevent WAL writes in certain condition. It should be enough to add an additional test in XLogInsertAllowed() with some new variable that is set when starting in binary upgrade mode, and a new function to disable it that will be emitted by pg_dump / pg_dumpall in binary upgrade mode. > What about c) where the bgworker are not loaded by default during binary > upgrade > mode / -b unless they explicitly set a bgw_flags (BGWORKER_BINARY_UPGRADE ?) > when they are required during pg_upgrade? As mentioned above +1 for not launching the bgworkers. Does anyone can think of a reason why some bgworker would really be needed during pg_upgrade, either on the source or the target cluster?
