On Tue, Jul 30, 2019 at 12:21 PM Thomas Munro wrote:
>
> On Tue, Jul 30, 2019 at 5:03 PM Dilip Kumar wrote:
> >
> > I think this idea is good for the DO time but during REDO time it will
> > not work as we will not have the transaction state. Having said that
> >
when it's shared
> memory. Copying data in tiny increments, with a bunch of branches, is
> even more expensive, especially when it's shared memory, especially when
> all that shared meory is locked at once.
My idea is, indeed of keeping all these fields duplicated in the
context, just allocate a single memory segment equal to the expected
record size (maybe the payload data can keep separate). Now, based on
uur_info pack all the field of UnpackedUndoRecord in that memory
segment. After that In InsertUndoData, we just need one call to
InsertUndoBytes for copying complete header in one shot and another
call for copying payload data. Does this sound reasonable to you?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
the last undo record.
I agree that in undoprocessing patch set we always need to fetch one
record so instead of repeating this pattern everywhere we can write
one function and move this sequence of calls in that function.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Jul 30, 2019 at 1:54 PM Dilip Kumar wrote:
>
> On Tue, Jul 30, 2019 at 12:21 PM Thomas Munro wrote:
> >
> > One data structure that could perhaps hold this would be
> > UndoLogTableEntry (the per-backend cache, indexed by undo log number,
> > with pretty
On Thu, Jul 18, 2019 at 4:58 PM Amit Kapila wrote:
>
> On Tue, Jul 16, 2019 at 2:20 PM Dilip Kumar wrote:
> >
>
> Few comments on the new patch:
>
> 1.
> Additionally,
> +there is a mechanism for multi-insert, wherein multiple records are prepared
> +and insert
On Fri, Jul 19, 2019 at 2:28 PM Amit Kapila wrote:
>
> On Thu, Jul 11, 2019 at 9:17 AM Dilip Kumar wrote:
> >
> > On Thu, Jul 11, 2019 at 12:38 AM Robert Haas wrote:
> > >
> > > I don't like the fact that undoaccess.c has a new global,
> > &g
>
>
> %s/the page the/the page then
>
> 12)
>
> /*
> * If the transaction's undo records are split across the undo logs. So
> * we need to update our own transaction header in the previous log.
> */
>
> double space between "to" and &
On Tue, Jul 30, 2019 at 12:21 PM Thomas Munro wrote:
>
> Hi Dilip,
>
> > commit 2f3c127b9e8bc7d27cf7adebff0a355684dfb94e
> > Author: Dilip Kumar
> > Date: Thu May 2 11:28:13 2019 +0530
> >
> >Provide interfaces to store and fetch undo recor
we only find this out by allocating? This seems like an API
> deficiency of the storage layer to me. The information is in the und log
> slot's metadata, no?
I agree with this. I think if Thomas agree we can provide an API in
undo log which can provide us this information before we do the actual
allocation.
>
>
> > + urec->uur_next = InvalidUndoRecPtr;
> > + UndoRecordSetInfo(urec);
> > + urec->uur_info |= UREC_INFO_TRANSACTION;
> > + urec->uur_info |= UREC_INFO_LOGSWITCH;
> > + size = UndoRecordExpectedSize(urec);
> > +
> > + /* Allocate space for the record. */
> > + if (InRecovery)
> > + {
> > + /*
> > + * We'll figure out where the space needs to be allocated by
> > + * inspecting the xlog_record.
> > + */
> > + Assert(context->alloc_context.persistence == UNDO_PERMANENT);
> > + urecptr = UndoLogAllocateInRecovery(&context->alloc_context,
> > +
> > XidFromFullTransactionId(txid),
> > +
> > size,
> > +
> > &need_xact_header,
> > +
> > &last_xact_start,
> > +
> > &prevlog_xact_start,
> > +
> > &prevlogurp);
> > + }
> > + else
> > + {
> > + /* Allocate space for writing the undo record. */
>
> That's basically the same comment as before the if.
Removed
>
>
> > + urecptr = UndoLogAllocate(&context->alloc_context,
> > + size,
> > +
> > &need_xact_header, &last_xact_start,
> > +
> > &prevlog_xact_start, &prevlog_insert_urp);
> > +
> > + /*
> > + * If prevlog_xact_start is a valid undo record pointer that
> > means
> > + * this transaction's undo records are split across undo logs.
> > + */
> > + if (UndoRecPtrIsValid(prevlog_xact_start))
> > + {
> > + uint16 prevlen;
> > +
> > + /*
> > + * If undo log is switch during transaction then we
> > must get a
>
> "is switch" is right.
This code is removed now.
>
> > +/*
> > + * Insert a previously-prepared undo records.
>
> s/a//
Fixed
>
>
> More tomorrow.
>
refer the latest patch at
https://www.postgresql.org/message-id/CAFiTN-uf4Bh0FHwec%2BJGbiLq%2Bj00V92W162SLd_JVvwW-jwREg%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
))
> > + {
> > + page = BufferGetPage(buffer);
> > + pagedata = (char *) page;
> > + }
> > +
> > + /*
> > + * Length if the previous undo record is store at the end of that
> > record
> > + * so just fe
+ {
> > + int len =
> > ucontext->urec_payload.urec_tuple_len;
> > +
> > + if (len > 0)
> > + {
> > + /* Insert tuple data. */
> > + if (!InsertUndoBytes((char *)
> > ucontext->urec_tupledata,
> > +
> >len, &writeptr, endptr,
> > +
> >&ucontext->already_processed,
> > +
> >&ucontext->partial_bytes))
> > + return;
> > + }
> > + ucontext->stage = UNDO_PACK_STAGE_UNDO_LENGTH;
> > + }
> > + /* fall through */
> > +
> > + case UNDO_PACK_STAGE_UNDO_LENGTH:
> > + /* Insert undo length. */
> > + if (!InsertUndoBytes((char *) &ucontext->undo_len,
> > +
> > sizeof(uint16), &writeptr, endptr,
> > +
> > &ucontext->already_processed,
> > +
> > &ucontext->partial_bytes))
> > + return;
> > +
> > + ucontext->stage = UNDO_PACK_STAGE_DONE;
> > + /* fall through */
> > +
> > + case UNDO_PACK_STAGE_DONE:
> > + /* Nothing to be done. */
> > + break;
> > +
> > + default:
> > + Assert(0); /* Invalid stage */
> > + }
> > +}
>
> I don't understand. The only purpose of this is that we can partially
> write a packed-but-not-actually-packed record onto a bunch of pages? And
> for that we have an endless chain of copy and pasted code calling
> InsertUndoBytes()? Copying data into shared buffers in tiny increments?
>
> If we need to this, what is the whole packed record format good for?
> Except for adding a bunch of functions with 10++ ifs and nearly
> identical code?
>
> Copying data is expensive. Copying data in tiny increments is more
> expensive. Copying data in tiny increments, with a bunch of branches, is
> even more expensive. Copying data in tiny increments, with a bunch of
> branches, is even more expensive, especially when it's shared
> memory. Copying data in tiny increments, with a bunch of branches, is
> even more expensive, especially when it's shared memory, especially when
> all that shared meory is locked at once.
>
>
> > +/*
> > + * Read the undo record from the input page to the unpack undo context.
> > + *
> > + * Caller can call this function multiple times until desired stage is
> > reached.
> > + * This will read the undo record from the page and store the data into
> > unpack
> > + * undo context, which can be later copied to unpacked undo record by
> > calling
> > + * FinishUnpackUndo.
> > + */
> > +void
> > +UnpackUndoData(UndoPackContext *ucontext, Page page, int starting_byte)
> > +{
> > + char *readptr = (char *) page + starting_byte;
> > + char *endptr = (char *) page + BLCKSZ;
> > +
> > + switch (ucontext->stage)
> > + {
> > + case UNDO_PACK_STAGE_HEADER:
>
> You know roughly what I'm thinking.
I have expressed my thought on this in last comment.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ad due to a large context with a lot of small
> allocations that then get individually freed).
Ok, I got your idea. I will analyze it further and work on this if
there is no problem.
That will make the
> sorting of undo a bit more CPU inefficient, because individual records
> will need to be partially unpacked for comparison, but I think that's
> going to be a far smaller loss than the win.
Right.
>
>
> - My reading of the current xact.c integration is that it's not workable
> as is. Undo is executed outside of a valid transaction state,
> exceptions aren't properly undone, logic would need to be duplicated
> to a significant degree, new kind of critical section.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 14, 2019 at 10:35 PM Andres Freund wrote:
>
> Hi,
>
> On 2019-08-14 14:48:07 +0530, Dilip Kumar wrote:
> > On Wed, Aug 14, 2019 at 12:27 PM Andres Freund wrote:
> > > - I think there's two fairly fundamental, and related, problems with
>
On Wed, Aug 14, 2019 at 2:48 PM Dilip Kumar wrote:
>
> On Wed, Aug 14, 2019 at 12:27 PM Andres Freund wrote:
> > I think that batch reading should just copy the underlying data into a
> > char* buffer. Only the records that currently are being used by
> > higher la
On Fri, Aug 16, 2019 at 10:56 AM Andres Freund wrote:
>
> Hi,
>
> On 2019-08-16 09:44:25 +0530, Dilip Kumar wrote:
> > On Wed, Aug 14, 2019 at 2:48 PM Dilip Kumar wrote:
> > >
> > > On Wed, Aug 14, 2019 at 12:27 PM Andres Freund wrote:
> >
> > &
ed to
somehow deal with the multi-insert code in the zheap because in that
code for inserting in a single page we write one undo record per range
if all the tuple which we are inserting on a single page are
interleaved. But, maybe we can handle that by just inserting one undo
record which can have multiple ranges.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
e caller need not worry
about setting them. So now you are suggesting to put other headers
also as structures in UnpackedUndoRecord. I as such don't have much
problem in doing that but I think initially Robert designed
UnpackedUndoRecord structure this way so it will be good if Robert
provides his opinion on this.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 14, 2019 at 10:35 PM Andres Freund wrote:
>
> Hi,
>
> On 2019-08-14 14:48:07 +0530, Dilip Kumar wrote:
> > On Wed, Aug 14, 2019 at 12:27 PM Andres Freund wrote:
> I don't think we can normally pin the undo buffers properly at that
> stage. Without knowin
On Tue, Aug 20, 2019 at 7:57 PM Robert Haas wrote:
>
> On Mon, Aug 19, 2019 at 2:04 AM Dilip Kumar wrote:
> > Currently, In UnpackedUndoRecord we store all members directly which
> > are set by the caller. We store pointers to some header which are
> > allocated internal
On Wed, Aug 21, 2019 at 9:04 PM Robert Haas wrote:
>
> On Wed, Aug 21, 2019 at 3:55 AM Dilip Kumar wrote:
> > I have already attempted that part and I feel it is not making code
> > any simpler than what we have today. For packing, it's fine because I
> > can pr
On Thu, Aug 22, 2019 at 9:58 AM Andres Freund wrote:
>
> Hi,
>
> On 2019-08-22 09:51:22 +0530, Dilip Kumar wrote:
> > We can not know the complete size of the record even by reading the
> > header because we have a payload that is variable part and payload
> > le
On Thu, Aug 22, 2019 at 10:24 AM Andres Freund wrote:
>
> Hi,
>
> On 2019-08-22 10:19:04 +0530, Dilip Kumar wrote:
> > On Thu, Aug 22, 2019 at 9:58 AM Andres Freund wrote:
> > >
> > > Hi,
> > >
> > > On 2019-08-22 09:51:22 +0530, Dilip Kum
On Thu, Aug 22, 2019 at 9:21 PM Dilip Kumar wrote:
>
> On Thu, Aug 22, 2019 at 7:34 PM Robert Haas wrote:
> >
> > On Thu, Aug 22, 2019 at 1:34 AM Dilip Kumar wrote:
> > > Yeah, we can handle the bulk fetch as you suggested and it will make
> > > it
On Thu, Aug 22, 2019 at 7:34 PM Robert Haas wrote:
>
> On Thu, Aug 22, 2019 at 1:34 AM Dilip Kumar wrote:
> > Yeah, we can handle the bulk fetch as you suggested and it will make
> > it a lot easier. But, currently while registering the undo request
> > (especially d
On Thu, Aug 22, 2019 at 9:55 PM Andres Freund wrote:
>
> Hi
>
> On August 22, 2019 9:14:10 AM PDT, Dilip Kumar wrote:
> > But, those requests will
> >ultimately be used for collecting the record by the bulk fetch. So if
> >we are planning to change the bulk fetch
function.
I have read 0003 and 0004 patch and there are few cosmetic comments.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Sep 3, 2019 at 12:11 PM Dilip Kumar wrote:
>
> On Fri, Aug 16, 2019 at 3:54 PM Jeevan Chalke
> wrote:
> >
> 0003:
> +/*
> + * When to send the whole file, % blocks modified (90%)
> + */
> +#define WHOLE_FILE_THRESHOLD 0.9
>
> How this thresh
; > builds. More importantly, I'm not very convinced that it's impossible
> > to hit the no-mapping case. The original proposal was to fall back
> > to current behavior (test the child-table permissions) if we couldn't
> > match the var to the top parent, and I think that that is still a
> > sane proposal.
>
> OK, I've removed the Assert. For child Vars that can't be translated
> to root parent's, permissions are checked with the child relation,
> like before.
Instead of falling back to the child, isn't it make more sense to
check the permissions on the parent upto which we could translate (it
may not be the root parent)?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Sep 5, 2019 at 2:48 PM Dilip Kumar wrote:
>
> On Thu, Sep 5, 2019 at 2:12 PM Amit Langote wrote:
>
> Thanks for the patch, I was almost about to press the send button with
> my patch. But, this looks similar to my version.
> >
> > On Wed, Sep 4, 2019
On Thu, Sep 5, 2019 at 3:26 PM Amit Langote wrote:
>
> On Thu, Sep 5, 2019 at 6:33 PM Dilip Kumar wrote:
> > /*
> > + * For inheritance child relations, we also need to remember
> > + * the root parent.
> > + */
> > + if (parent->rtekind == RTE_RELAT
ough (i.e. 8192) then there is no problem, but
when you set it to 4096, in that case, the hashm_mapp of the meta page
is overwriting the special space of the meta page. That's the reason
its showing corrupted page while checking the hash_page.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 29, 2018 at 3:39 PM, Dilip Kumar wrote:
> On Tue, Aug 28, 2018 at 8:33 PM, Bernd Helmle wrote:
>> Am Dienstag, den 28.08.2018, 11:21 +0200 schrieb Peter Eisentraut:
>>> This is reproducible with PG11 and PG12:
>>>
>>> initdb -k data
>>&g
t; 3. Andres also suggested that high frequency free page list access
> might be quite contended in the "best case" described above. I'll look
> into that.
> 4. Someone said that segment sizes probably shouldn't be hard coded
> (cf WAL experience).
>
> I also l
On Fri, Aug 31, 2018 at 3:08 PM, Dilip Kumar wrote:
> Hello hackers,
>
> As Thomas has already mentioned upthread that we are working on an
> undo-log based storage and he has posted the patch sets for the lowest
> layer called undo-log-storage.
>
> This is the next layer whi
Min(BLCKSZ / 8, 1024)
then for BLCKSZ 8K and bigger, it will remain the same value where it
does not overrun. And, for the small BLCKSZ, I think it will give
sufficient space for the hash map. If the BLCKSZ is 1K then the sizeof
(HashMetaPageData) + sizeof (HashPageOpaque) = 968 which is very close
to the BLCKSZ.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sun, Sep 2, 2018 at 12:18 AM, Thomas Munro
wrote:
> On Fri, Aug 31, 2018 at 10:24 PM Dilip Kumar
> wrote:
>> On Fri, Aug 31, 2018 at 3:08 PM, Dilip Kumar wrote:
>> > As Thomas has already mentioned upthread that we are working on an
>> > undo-log based stora
On Mon, Sep 3, 2018 at 8:37 AM, Amit Kapila wrote:
> On Sat, Sep 1, 2018 at 10:28 AM Dilip Kumar wrote:
>>
>> On Sat, Sep 1, 2018 at 8:22 AM, Robert Haas wrote:
>> > On Thu, Aug 30, 2018 at 7:27 AM, Amit Kapila
>> > wrote:
>> >
>> > I wo
grouping_planner(root, false, tuple_fraction);
I think we can add some comments to explain if the target rel itself
is partitioned rel then why
we can directly go to the grouping planner.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Sep 4, 2018 at 10:14 AM, Amit Kapila wrote:
> On Mon, Sep 3, 2018 at 2:44 PM Dilip Kumar wrote:
>> On Mon, Sep 3, 2018 at 8:37 AM, Amit Kapila wrote:
>> > On Sat, Sep 1, 2018 at 10:28 AM Dilip Kumar wrote:
>> >>
>> >> I think if we compute wit
On Tue, Sep 4, 2018 at 10:49 AM, Dilip Kumar wrote:
> On Tue, Sep 4, 2018 at 10:14 AM, Amit Kapila wrote:
>> On Mon, Sep 3, 2018 at 2:44 PM Dilip Kumar wrote:
>>> On Mon, Sep 3, 2018 at 8:37 AM, Amit Kapila wrote:
>>> > On Sat, Sep 1, 2018 at 10:28 AM Dilip Kumar
On Tue, Sep 4, 2018 at 12:44 PM, Amit Langote
wrote:
> Hi Dilip,
>
> Thanks for taking a look.
>
> On 2018/09/03 20:57, Dilip Kumar wrote:
>> The idea looks interesting while going through the patch I observed
>> this comment.
>>
>> /*
>> * inheritan
28727c4b48639c0e12bf2f70a766b910".
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
.org/message-id/CAD__OujRZEjE5y3vfmmZmSSr3oYGZSHRxwDwF7kyhBHB2BpW_g%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
only the
+ * partitions that are not pruned.
+ */
prunin/pruning
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
access the statistics of the table if a user has privilege on
its parent table?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Oct 22, 2018 at 11:22 AM David Fetter wrote:
>
> On Mon, Oct 22, 2018 at 11:10:09AM +0530, Dilip Kumar wrote:
> > As part of the security fix
> > (e2d4ef8de869c57e3bf270a30c12d48c2ce4e00c), we have restricted the
> > users from accessing the statistics of the t
On Mon, Oct 22, 2018 at 12:05 PM Amit Langote
wrote:
>
> Hi,
>
> On 2018/10/22 14:41, Stephen Frost wrote:
> > Greetings,
> >
> > * Dilip Kumar (dilipbal...@gmail.com) wrote:
> >> As part of the security fix
> >> (e2d4ef8de869c57e3bf270a30c12d48c2
On Mon, Oct 22, 2018 at 7:16 PM Tom Lane wrote:
>
> Dilip Kumar writes:
> > As part of the security fix
> > (e2d4ef8de869c57e3bf270a30c12d48c2ce4e00c), we have restricted the
> > users from accessing the statistics of the table if the user doesn't
> > h
On Mon, Oct 22, 2018 at 7:40 PM David Fetter wrote:
>
> On Mon, Oct 22, 2018 at 04:43:52PM +0530, Dilip Kumar wrote:
> > On Mon, Oct 22, 2018 at 11:22 AM David Fetter wrote:
> > >
> > > On Mon, Oct 22, 2018 at 11:10:09AM +0530, Dilip Kumar wrote:
>
;acl_ok accordingly.
Additionally, for getting the parent RTI we need to traverse
"root->append_rel_list". Another alternative could be that we can add
parent_rti member in RelOptInfo structure.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Oct 26, 2018 at 1:12 PM Amit Langote
wrote:
>
> On 2018/10/25 19:54, Dilip Kumar wrote:
> > On Mon, Oct 22, 2018 at 7:47 PM Tom Lane wrote:
> >> Amit Langote writes:
> >>> But maybe for the case under question, that's irrelevant, because
&g
On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar wrote:
>
> On Fri, Oct 26, 2018 at 1:12 PM Amit Langote
> wrote:
> >
> > On 2018/10/25 19:54, Dilip Kumar wrote:
> > > On Mon, Oct 22, 2018 at 7:47 PM Tom Lane wrote:
> > >> Amit Langote writes:
> > &
On Mon, Oct 29, 2018 at 2:53 PM Amit Langote
wrote:
>
> Thank you for creating the patch.
>
> On 2018/10/28 20:35, Dilip Kumar wrote:
> > On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar wrote:
> >> On Fri, Oct 26, 2018 at 1:12 PM Amit Langote wrote:
> >>>
added a patch to fix the
> same.
>
I have included your fix in the latest version of the undo-worker patch[1]
[1]
https://www.postgresql.org/message-id/flat/CAFiTN-sYQ8r8ANjWFYkXVfNxgXyLRfvbX9Ee4SxO9ns-OBBgVA%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Nov 5, 2018 at 5:09 PM Dilip Kumar wrote:
>
> On Mon, Sep 3, 2018 at 11:26 AM Dilip Kumar wrote:
>
> Thomas has already posted the latest version of undo log patches on
> 'Cleaning up orphaned files using undo logs' thread[1]. So I have
> rebased the undo
please find the updated patch.
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
0004-undo-interface-test-v5.patch
Description: Binary data
0003-undo-interface-v5.patch
Description: Binary data
array index.
And, later we can add some wrapper over get_partitions_for_key such
that it can recursively traverse the non-leaf partitions (something
like get_partition_for_tuple does as Amit mentioned.). And, ultimately
gives the output as a list of leaf partition's indexes. Then we c
I think BitmapHeapScan check whether dsa is valid or not if DSA is not
valid then it should assume it's non-parallel plan.
Attached patch should fix the issue.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
bug_fix_in_pbhs_when_dsa_not_initialized.patch
Description: Binary data
On Tue, Nov 28, 2017 at 7:13 PM, Robert Haas wrote:
> On Tue, Nov 28, 2017 at 2:32 AM, Dilip Kumar
> wrote:
> > I think BitmapHeapScan check whether dsa is valid or not if DSA is not
> > valid then it should assume it's non-parallel plan.
> >
> > Attached
st.
>
I have looked into this crash, seems like below assert in exec_stmt_execsql
is not correct
*case* SPI_OK_REWRITTEN:
Assert(!stmt->mod_stmt);
In this issue first time execution of select en1(1); statement, stmt->mod_stmt
is set to true for the insert query inside the function. Then b
olAConst(bool state, int location)
{
Node *n = makeBoolAConstNoCast(state, location);
return makeTypeCast(n, SystemTypeName("bool"), -1);
}
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Dec 11, 2017 at 4:33 PM, Dilip Kumar wrote:
> On Mon, Dec 11, 2017 at 3:54 PM, tushar
> wrote:
>
>> Hi,
>>
>> While testing something , I found that even after rule has dropped not
>> able to insert data and in an another scenario , there is a Crash/
&
int was to make sure
> certain existing notifications will be preserved - sure, why not.
IMHO, after having 64-bit XID this WARNING doesn't really make sense.
Those warnings exist because those limits were problematic for 32-bit
xid but now it is not so I think we should not have such warnings.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
the index vacuum when we think there are enough dead items so that
the index vacuum makes sense[1].
[1]
https://www.postgresql.org/message-id/CA%2BTgmoZgapzekbTqdBrcH8O8Yifi10_nB7uWLB8ajAhGL21M6A%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Nov 24, 2022 at 9:39 PM Justin Pryzby wrote:
> On Thu, Nov 24, 2022 at 08:52:16PM +0530, Dilip Kumar wrote:
> > but now you will have one gigantic index and which will be vacuumed
> > every time we vacuum any of the partitions.
>
> This patch isn't implemen
On Fri, Nov 25, 2022 at 8:49 AM Dilip Kumar wrote:
>
> On Thu, Nov 24, 2022 at 9:39 PM Justin Pryzby wrote:
> > On Thu, Nov 24, 2022 at 08:52:16PM +0530, Dilip Kumar wrote:
> > > but now you will have one gigantic index and which will be vacuumed
> > > every time w
k into the pg_subtrans SLRU and
fetch the top xid and relook again into the xip array. It will be
more costly in cases where we do not find xid in the xip array because
then we will have to search this array twice but I think looking into
this array is much cheaper than directly accessing SLRU.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ransaction should have never been streamed.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
From 20cc1084c4943bdaf23753f2a7d9add22097ed95 Mon Sep 17 00:00:00 2001
From: Dilip Kumar
Date: Fri, 25 Nov 2022 13:11:44 +0530
Subject: [PATCH v1] Fix thinko in when to stream a transaction
on that will be used by streaming as well as by DecodeCommit to
decide on whether to skip or not.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, Nov 26, 2022 at 12:15 PM Amit Kapila wrote:
>
> On Fri, Nov 25, 2022 at 5:38 PM Amit Kapila wrote:
> >
> > On Fri, Nov 25, 2022 at 1:35 PM Dilip Kumar wrote:
> > >
> > > During DecodeCommit() for skipping a transaction we use ReadRecPtr to
> > &
l_change(toptxn)) &&
> - rbtxn_is_serialized(txn))
> + rbtxn_is_serialized(txn) &&
> + rbtxn_has_streamable_change(txn))
> ReorderBufferStreamTXN(rb, toptxn);
> }
You are right we need this in ReorderBufferProcessPartialChange(
On Mon, Nov 28, 2022 at 3:19 PM Dilip Kumar wrote:
>
> On Mon, Nov 28, 2022 at 1:46 PM shiy.f...@fujitsu.com
> wrote:
> >
> > Thanks for your patch.
> >
> > I saw that the patch added a check when selecting largest transaction, but
> > in
> > add
ase for
> this patch. I think before this patch if the size of DDL changes in a
> transaction exceeds logical_decoding_work_mem, the empty streams will
> be output in the plugin but after this patch, there won't be any such
> stream.
Yes, we can do that, I will make these two changes.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Dec 5, 2022 at 9:21 AM Dilip Kumar wrote:
>
> On Mon, Dec 5, 2022 at 8:59 AM Amit Kapila wrote:
> >
> > On Sun, Dec 4, 2022 at 5:14 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Saturday, December 3, 2022 7:37 PM Amit Kapila
> >
On Wed, Dec 7, 2022 at 9:28 AM Amit Kapila wrote:
>
> On Tue, Dec 6, 2022 at 11:55 AM shiy.f...@fujitsu.com
> wrote:
> >
> > On Mon, Dec 5, 2022 6:57 PM Amit Kapila wrote:
> > >
> > > On Mon, Dec 5, 2022 at 3:41 PM Dilip Kumar wrote:
> > > >
XX = true/false
> logical_replication_dev_YYY = true/false
> logical_replication_dev_ZZZ = true/false
>
Even I agree that usability wise keeping them independent is better.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
uot;force_stream" then it will either stream or serialize but I
guess we do not want to force serialize based on this parameter.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Returns a record of information about the subtransactions of the
> > backend
> > +with the specified ID.
> > +The fields returned are subxact_count, which
> > +identifies the number of active subtransaction and
> > +subxact_overflow, which s
On Tue, Dec 20, 2022 at 2:32 AM Robert Haas wrote:
>
> On Mon, Dec 19, 2022 at 3:48 PM Ted Yu wrote:
> > It seems the comment for `backend_subxact_overflowed` missed a word.
> >
> > Please see the patch.
>
> Committed this fix, thanks.
Thanks, Robert!
--
Reg
not stream for some reason e.g. partial change
then we serialize. And I feel your previous patch was mimicking the
same behavior for each change. Now in the new patch, we will try to
stream and if we can not we will queue the change so I feel we are
creating a new patch that actually doesn't exist without the force
mode.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
e above.
>
> Thoughts?
I think option 2 makes sense because stream/serialize are two related
options and both are dependent on the same parameter
(logical_decoding_work_mem) so having a single know is logical. And
another GUC for serializing logical apply.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
; LGTM as well. I'll push this on Monday.
>
The patch looks good to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ication.
>
> WALRead() callers are smart enough to take the flushed bytes only.
> Although they read the whole WAL page, they calculate the valid bytes.
Right
On first read the patch looks good, although it needs some more
thoughts on 'XXX' comments in the
ot spool the data in file? How are we going to get the
commit time of the transaction without applying the changes?
5.
+/*
+ * The following operations use these special functions to detect
+ * overflow. Number of ms per informed days.
+ */
This comment doesn't make m
r the worker to catch up
with the changes then it better to again take advantage of using
memory. Otherwise, in this case, we are just wasting resources
(worker/shared memory queue) but still writing in the file.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Dec 26, 2022 at 2:44 PM Amit Kapila wrote:
>
> On Mon, Dec 26, 2022 at 2:12 PM Dilip Kumar wrote:
> >
> > On Fri, Dec 23, 2022 at 9:16 PM Takamichi Osumi (Fujitsu)
> > wrote:
> > >
> >
> > 4.
> >
> > + * Although the delay
On Tue, Dec 27, 2022 at 9:15 AM Amit Kapila wrote:
>
> On Mon, Dec 26, 2022 at 7:35 PM Dilip Kumar wrote:
> >
> > In the commit message, there is a statement like this
> >
> > "However, if the leader apply worker times out while attempting to
> > send a
ou mean to say that with logical the shutdown will be
delayed until all the changes are applied on the subscriber but the
same is not true for physical standby? Is it because on physical
standby we flush the WAL before applying?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
if we determine that there's still more data to send or we are in
+ * the physical replication more, this function will return control to the
+ * caller.
I think in this comment you meant to say
1. "or we are in physical replication mode and all WALs are not yet replicated"
On Mon, May 4, 2020 at 5:16 PM Amit Kapila wrote:
>
> On Fri, May 1, 2020 at 8:41 PM Dilip Kumar wrote:
> >
> 5. Shouldn't we add a check in table_scan_sample_next_block and
> table_scan_sample_next_tuple APIs as well?
I am not sure that we need to do that, Because gene
On Tue, May 5, 2020 at 10:25 AM Amit Kapila wrote:
>
> On Tue, May 5, 2020 at 9:27 AM Dilip Kumar wrote:
> >
> > On Mon, May 4, 2020 at 5:16 PM Amit Kapila wrote:
> > >
> > > On Fri, May 1, 2020 at 8:41 PM Dilip Kumar wrote:
> > &g
On Sun, May 10, 2020 at 11:17 PM Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> > On Sat, Apr 11, 2020 at 03:17:25PM +0530, Dilip Kumar wrote:
> >
> > Some more comments...
>
> Thanks for reviewing. Since this patch took much longer than I expected,
> it&#x
se questions, but I felt that
> they weren't the most important problems to solve first.
>
> What do you all think?
The overall idea looks quite nice. I had a look at some of the
patches at least 0005 and 0006. At first look, I have one comment.
+/*
+ * Each archive is set as a separate stream of COPY data, and thus begins
+ * with a CopyOutResponse message.
+ */
+static void
+bbsink_libpq_begin_archive(bbsink *sink, const char *archive_name)
+{
+ SendCopyOutResponse();
+}
Some of the bbsink_libpq_* functions are directly calling pq_* e.g.
bbsink_libpq_begin_backup whereas others are calling SendCopy*
functions and therein those are calling pq_* functions. I think
bbsink_libpq_* function can directly call pq_* functions instead of
adding one more level of the function call.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, May 13, 2020 at 1:56 AM Robert Haas wrote:
>
> On Tue, May 12, 2020 at 4:32 AM Dilip Kumar wrote:
> > Some of the bbsink_libpq_* functions are directly calling pq_* e.g.
> > bbsink_libpq_begin_backup whereas others are calling SendCopy*
> > functions and there
gt;> build_replindex_scan_key needs to be updated.
>>
>> * This is not generic routine, it expects the idxrel to be replication
>> * identity of a rel and meet all limitations associated with that.
>>
> It is implicit that a primary key can be a replica identity so I think this
> comment is fine.
I like your idea of modifying the assert instead of completely removing.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, May 12, 2020 at 4:39 PM Amit Kapila wrote:
>
> On Thu, May 7, 2020 at 6:17 PM Dilip Kumar wrote:
> >
> > On Tue, May 5, 2020 at 7:13 PM Dilip Kumar wrote:
> >
> > I have fixed one more issue in 0010 patch. The issue was that once
> > the transaction
On Mon, May 11, 2020 at 4:55 PM Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> > On Mon, May 11, 2020 at 04:04:00PM +0530, Dilip Kumar wrote:
> >
> > > > +static inline bool
> > > > +_bt_scankey_within_page(IndexScanDesc scan, BTScanInsert
On Wed, May 13, 2020 at 4:50 PM Amit Kapila wrote:
>
> On Wed, May 13, 2020 at 11:35 AM Dilip Kumar wrote:
> >
> > On Tue, May 12, 2020 at 4:39 PM Amit Kapila wrote:
> > >
> > >
> > > v20-0003-Ext
wise, we might have to worry that those
triggers could do something on the primary table before we check the
constraint. I am not sure if there are any other factors that I am
missing.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, May 13, 2020 at 4:50 PM Amit Kapila wrote:
>
> On Wed, May 13, 2020 at 11:35 AM Dilip Kumar wrote:
> >
> > On Tue, May 12, 2020 at 4:39 PM Amit Kapila wrote:
> > >
> > >
> > > v20-0003-Ext
1101 - 1200 of 1584 matches
Mail list logo
