On Thu, Oct 28, 2021 at 8:15 AM Amit Kapila wrote:
>
> On Wed, Oct 27, 2021 at 4:39 PM Dilip Kumar wrote:
> >
> > On Tue, Oct 26, 2021 at 9:19 AM Amit Kapila wrote:
> > >
> > > >
> > > > Thanks, both your patches look good to me except that we need to
> > > > remove the sentence related to the r
On Wed, Oct 27, 2021 at 4:39 PM Dilip Kumar wrote:
>
> On Tue, Oct 26, 2021 at 9:19 AM Amit Kapila wrote:
> >
> > >
> > > Thanks, both your patches look good to me except that we need to
> > > remove the sentence related to the revert of ade24dab97 from the
> > > commit message. I think we should
On Tue, Oct 26, 2021 at 9:19 AM Amit Kapila wrote:
>
> On Mon, Oct 25, 2021 at 4:21 PM Amit Kapila wrote:
> >
> > On Thu, Oct 21, 2021 at 11:20 AM Dilip Kumar wrote:
> > >
> > > On Thu, Oct 21, 2021 at 9:11 AM Amit Kapila
> > > wrote:
> > > >
> > >
> > > v5-0001, incorporates all the comment f
On Mon, Oct 25, 2021 at 4:21 PM Amit Kapila wrote:
>
> On Thu, Oct 21, 2021 at 11:20 AM Dilip Kumar wrote:
> >
> > On Thu, Oct 21, 2021 at 9:11 AM Amit Kapila wrote:
> > >
> >
> > v5-0001, incorporates all the comment fixes suggested by Alvaro. and
> > 0001 is an additional patch which moves
>
On Thu, Oct 21, 2021 at 11:20 AM Dilip Kumar wrote:
>
> On Thu, Oct 21, 2021 at 9:11 AM Amit Kapila wrote:
> >
>
> v5-0001, incorporates all the comment fixes suggested by Alvaro. and
> 0001 is an additional patch which moves
> MarkCurrentTransactionIdLoggedIfAny(), out of the critical section.
On Thu, Oct 21, 2021 at 9:11 AM Amit Kapila wrote:
>
> On Wed, Oct 20, 2021 at 8:49 PM Dilip Kumar wrote:
> >
> > On Wed, Oct 20, 2021 at 7:09 PM Alvaro Herrera
> > wrote:
> > >
> > > Does MarkTopTransactionIdLogged() have to be inside XLogInsertRecord's
> > > critical section?
> >
> > I think
On Wed, Oct 20, 2021 at 8:49 PM Dilip Kumar wrote:
>
> On Wed, Oct 20, 2021 at 7:09 PM Alvaro Herrera
> wrote:
> >
> > Does MarkTopTransactionIdLogged() have to be inside XLogInsertRecord's
> > critical section?
>
> I think this function is doing somewhat similar things to what we are
> doing in
On Wed, Oct 20, 2021 at 9:46 PM Alvaro Herrera wrote:
>
> On 2021-Oct-20, Dilip Kumar wrote:
>
> > On Wed, Oct 20, 2021 at 7:09 PM Alvaro Herrera
> > wrote:
>
> > > In IsTopTransactionIdLogPending(), I suggest to reorder the tests so
> > > that the faster ones are first -- or at least, the last
On 2021-Oct-20, Dilip Kumar wrote:
> On Wed, Oct 20, 2021 at 7:09 PM Alvaro Herrera
> wrote:
> > In IsTopTransactionIdLogPending(), I suggest to reorder the tests so
> > that the faster ones are first -- or at least, the last one should be at
> > the top, so in some cases we can return without
On Wed, Oct 20, 2021 at 7:09 PM Alvaro Herrera wrote:
>
> API-wise, this seems a good improvement and it brings a lot of clarity
> to what is really going on. Thanks for working on it.
>
> Some minor comments:
Thanks for the review, most of the comments look fine, and will work
on those, but I t
API-wise, this seems a good improvement and it brings a lot of clarity
to what is really going on. Thanks for working on it.
Some minor comments:
Please do not revert the comment change in xlogrecord.h. It is not
wrong. The exceptions mentioned are values 252-255, so "a few" is
better than "a
On Wed, Oct 20, 2021 at 4:17 PM Amit Kapila wrote:
>
> On Wed, Oct 20, 2021 at 10:25 AM Dilip Kumar wrote:
> >
> > I have one comment here, basically, you have changed the function name
> > to "IsTopTransactionIdLogged", but it still behaves like
> > IsTopTransactionIdLogPending. Now with the ne
On Wed, Oct 20, 2021 at 10:25 AM Dilip Kumar wrote:
>
> On Mon, Oct 18, 2021 at 10:48 AM Amit Kapila wrote:
>
> >
> > Today, I have looked at this patch again and slightly changed a
> > comment, one of the function name and variable name. Do, let me know
> > if you or others have any suggestions
On Mon, Oct 18, 2021 at 10:48 AM Amit Kapila wrote:
>
> Today, I have looked at this patch again and slightly changed a
> comment, one of the function name and variable name. Do, let me know
> if you or others have any suggestions for better names or otherwise? I
> think we should backpatch this
On Thu, Oct 7, 2021 at 10:46 AM Amit Kapila wrote:
>
> On Sat, Oct 2, 2021 at 4:16 PM Dilip Kumar wrote:
> >
> > On Fri, Oct 1, 2021 at 6:24 PM Alvaro Herrera
> > wrote:
> > >
> > > On 2021-Oct-01, Amit Kapila wrote:
> >
> > > I think a straight standalone variable (probably a static boolean in
On Sat, Oct 2, 2021 at 4:16 PM Dilip Kumar wrote:
>
> On Fri, Oct 1, 2021 at 6:24 PM Alvaro Herrera wrote:
> >
> > On 2021-Oct-01, Amit Kapila wrote:
>
> > I think a straight standalone variable (probably a static boolean in
> > xloginsert.c) might be less confusing.
>
> I have written two patche
On Sat, Oct 2, 2021 at 6:46 AM Dilip Kumar wrote:
> I have written two patches, Approach1 is as you described using a
> static boolean and Approach2 as a local variable to XLogAssembleRecord
> as described by Amit, attached both of them for your reference.
> IMHO, either of these approaches looks
On Fri, Oct 1, 2021 at 6:23 PM Alvaro Herrera wrote:
>
> On 2021-Oct-01, Amit Kapila wrote:
>
> > AFAICS, there are two possibilities w.r.t global variables: (a) use
> > curinsert_flags which we are doing now, (b) another is to introduce a
> > new global variable, set it after we make the associat
On Sun, Oct 3, 2021 at 5:05 PM Dilip Kumar wrote:
>
> On Sat, Oct 2, 2021 at 8:10 PM Alvaro Herrera wrote:
> >
> > On 2021-Oct-02, Dilip Kumar wrote:
> >
> > > I have written two patches, Approach1 is as you described using a
> > > static boolean and Approach2 as a local variable to XLogAssembleR
On Sat, Oct 2, 2021 at 8:10 PM Alvaro Herrera wrote:
>
> On 2021-Oct-02, Dilip Kumar wrote:
>
> > I have written two patches, Approach1 is as you described using a
> > static boolean and Approach2 as a local variable to XLogAssembleRecord
> > as described by Amit, attached both of them for your re
On 2021-Oct-02, Dilip Kumar wrote:
> I have written two patches, Approach1 is as you described using a
> static boolean and Approach2 as a local variable to XLogAssembleRecord
> as described by Amit, attached both of them for your reference.
> IMHO, either of these approaches looks cleaner.
Thank
On Fri, Oct 1, 2021 at 6:24 PM Alvaro Herrera wrote:
>
> On 2021-Oct-01, Amit Kapila wrote:
> I think a straight standalone variable (probably a static boolean in
> xloginsert.c) might be less confusing.
I have written two patches, Approach1 is as you described using a
static boolean and Approac
On Fri, Oct 1, 2021 at 8:53 AM Alvaro Herrera wrote:
> I think a straight standalone variable (probably a static boolean in
> xloginsert.c) might be less confusing.
+1.
> ... so, reading the xact.c code again, TransactionState->assigned really
> means "whether the subXID-to-topXID association ha
On 2021-Oct-01, Amit Kapila wrote:
> AFAICS, there are two possibilities w.r.t global variables: (a) use
> curinsert_flags which we are doing now, (b) another is to introduce a
> new global variable, set it after we make the association, and then
> reset it after we mark SubTransaction assigned an
On Fri, Oct 1, 2021 at 12:32 AM Robert Haas wrote:
>
> On Thu, Sep 30, 2021 at 6:08 AM Amit Kapila wrote:
> > I think we can do better than using XLOG_INCLUDE_XID flag in the
> > record being inserted. We need this flag so that we can mark
> > SubTransaction assigned after XLogInsertRecord() is
On Thu, Sep 30, 2021 at 6:08 AM Amit Kapila wrote:
> I think we can do better than using XLOG_INCLUDE_XID flag in the
> record being inserted. We need this flag so that we can mark
> SubTransaction assigned after XLogInsertRecord() is successful. We
> can instead output a flag (say sub_xact_assig
On Wed, Sep 29, 2021 at 8:50 PM Robert Haas wrote:
>
> On Tue, Sep 21, 2021 at 6:48 PM Alvaro Herrera
> wrote:
> > Document XLOG_INCLUDE_XID a little better
> >
> > I noticed that commit 0bead9af484c left this flag undocumented in
> > XLogSetRecordFlags, which led me to discover that the flag do
On Tue, Sep 21, 2021 at 6:48 PM Alvaro Herrera wrote:
> Document XLOG_INCLUDE_XID a little better
>
> I noticed that commit 0bead9af484c left this flag undocumented in
> XLogSetRecordFlags, which led me to discover that the flag doesn't
> actually do what the one comment on it said it does. Impro
28 matches
Mail list logo