public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Error "initial slot snapshot too large" in create replication slot
Date: Fri, 01 Apr 2022 14:44:30 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAFiTN-s_PYicXeb6+uZkERqATpZLzpfsg0qP6G_GMPhu=rgxUA@mail.gmail.com>
	<[email protected]>

At Sun, 13 Feb 2022 17:35:38 +0300, Yura Sokolov <[email protected]> wrote in 
> В Пн, 07/02/2022 в 13:52 +0530, Dilip Kumar пишет:
> > Right. I think the patch looks fine to me.
> > 
> 
> Good day.
> 
> I've looked to the patch. Personally I'd prefer dynamically resize
> xip array. But I think there is issue with upgrade if replica source
> is upgraded before destination, right?

I don't think it is relevant.  I think we don't convey snapshot via
streaming replication.  But I think that expanding xip or subxip is
wrong, since it is tied with ProcArray structure. (Even though we
abuse the arrays in some situations, like this).

> Concerning patch, I think more comments should be written about new
> usage case for `takenDuringRecovery`. May be this field should be renamed
> at all?

I don't feel the need to rename it so much. It just signals that "this
snapshot is in the form for recovery".  The more significant reason is
that I don't come up better name:p

And the comment is slightly modified and gets a pointer to detailed
comment.

+	 * Although this snapshot is not acutally taken during recovery, all XIDs
+	 * are stored in subxip. See GetSnapshotData for the details of that form
+	 * of snapshot.


> And there are checks for `takenDuringRecovery` in `heapgetpage` and
> `heapam_scan_sample_next_tuple`. Are this checks affected by the change?
> Neither the preceding discussion nor commit message answer me.

The snapshot works correctly, but for the heapgetpage case, it foces
all_visible to be false.  That unnecessarily prevents visibility check
from skipping.

An annoying thing in SnapBuildInitialSnapshot is that we don't know
the number of xids before looping over the xid range, and we don't
want to bother sorting top-level xids and subxids unless we have to do
so.

Is it better that we hassle in SnapBuildInitialSnapshot to create a
!takenDuringRecovery snapshot?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center






view thread (18+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Error "initial slot snapshot too large" in create replication slot
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox