On Tue, Nov 28, 2017 at 7:13 PM, Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Nov 28, 2017 at 2:32 AM, Dilip Kumar <dilipbal...@gmail.com>
> wrote:
> > I think BitmapHeapScan check whether dsa is valid or not if DSA is not
> > valid then it should
d this
> code thinks that the DSA is available. Oops.
>
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
list of leaf partition's indexes. Then we can
find the sub-plan number by looking into the array.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
t Roman grave inscription +
>
>
I think the below commit is missed in the release notes?
5edc63bda68a77c4d38f0cbeae1c4271f9ef4100
Committer: Robert Haas <rh...@postgresql.org> 2017-11-10 13:50:50
Account for the effect of lossy pages when costing bitmap scans.
Dilip Kumar, revi
e. I am not
sure you are already doing this or its an open item?
>
>
> [1] https://github.com/HypoPG/hypopg
>
> Best regards,
>
> Yuzuko Hosoya
> NTT Open Source Software Center
>
>
>
>
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
;
COMMIT
UPDATE 1
postgres=# select * from pa_target ;
key | val
-+-
2 | initial1 updated by update2 --> session1's update is overwritten.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar wrote:
> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar
> wrote:
>
>> Attached is a rebased patch version. Also included it in the upcoming
>> commitfest :
>> https://commitfest.postgresql.org/18/1660/
>>
>> In
ata; /* dump sequence data even in schema-only mode */
+ int do_nothing;
} DumpOptions;
The new structure member appears out of place, can you move up along
with other "command-line long options" ?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
uld we also create a test case where we can verify that some
unnecessary or duplicate triggers are not executed? For example, in
the above trigger function, we can maintain a count in some table (e.g
how many time delete trigger got executed) and after test over we can
verify the same.
--
Reg
On Mon, Jun 11, 2018 at 10:19 AM, Amit Khandekar
wrote:
> On 8 June 2018 at 16:44, Dilip Kumar wrote:
> > On Fri, Jun 8, 2018 at 2:23 PM, Dilip Kumar
> wrote:
> >>
> >> On Tue, Jun 5, 2018 at 8:03 PM, Amit Khandekar
> >> wrote:
> >>>
>
Congratulations to all.
On Sat, 2 Jun 2018, 08:20 Kuntal Ghosh, wrote:
> Congratulations all.
>
> On Sat, 2 Jun 2018 at 2:35 AM, Tom Lane wrote:
>
>> The core team is pleased to announce the appointment of seven
>> new Postgres committers:
>>
>> Etsuro Fujita
>> Peter Geoghegan
>> Amit Kapila
On Mon, Dec 11, 2017 at 4:33 PM, Dilip Kumar <dilipbal...@gmail.com> wrote:
> On Mon, Dec 11, 2017 at 3:54 PM, tushar <tushar.ah...@enterprisedb.com>
> wrote:
>
>> Hi,
>>
>> While testing something , I found that even after rule has dropped not
>> ab
On Mon, Jun 18, 2018 at 6:19 PM, Amit Khandekar wrote:
> On 18 June 2018 at 17:56, Amit Kapila wrote:
>> On Mon, Jun 18, 2018 at 11:28 AM, Dilip Kumar wrote:
>>> Should we also create a test case where we can verify that some
>>> unnecessary or duplicate triggers ar
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
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 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 la
requency 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 learned in other sessions that there are other access ma
xElem" of the index
key. So if any field of the "IndexElem" is not same then it will be
considered as non-intersecting and in this example, the ORDER is not
matching.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
rt where a is null and b = 100;
QUERY PLAN
--
Append (cost=0.00..35.51 rows=1 width=12)
-> Seq Scan on public.prt_def (cost=0.00..35.50 rows=1 width=12)
Output: prt_def.a, prt_def.b, prt
On Wed, Jul 11, 2018 at 4:20 PM, Dilip Kumar wrote:
> On Wed, Jul 11, 2018 at 3:06 PM, Ashutosh Bapat
> wrote:
>> Hi,
>> Consider following test case.
>> create table prt (a int, b int, c int) partition by range(a, b);
>> create table prt_p1 partition of prt
On Wed, Jul 11, 2018 at 5:36 PM, amul sul wrote:
> On Wed, Jul 11, 2018 at 5:10 PM Dilip Kumar wrote:
>>
> I am not sure that I have understand the following comments
> 11 +* Generate one prune step for the information derived from IS NULL,
> 12 +* if any. To pru
and added regression tests.
Thanks, Your changes look fine to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
e95a1 in CheckpointerMain () at checkpointer.c:383
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
fix_fpwupdate.patch
Description: Binary data
On Fri, Mar 23, 2018 at 1:19 PM, Michael Paquier <mich...@paquier.xyz>
wrote:
> On Tue, Mar 20, 2018 at 12:00:47PM +0530, Dilip Kumar wrote:
> > Yeah, you are right. Fixed.
>
> So I have been spending a couple of hours playing with your patch, and
> tested various confi
On Tue, Mar 20, 2018 at 11:26 AM, Michael Paquier <mich...@paquier.xyz>
wrote:
> On Tue, Mar 20, 2018 at 10:43:55AM +0530, Dilip Kumar wrote:
> > I think like WALWriterProcess, we need to call InitXLogInsert for the
> > CheckpointerProcess as well as for the BgWriterProces
heckpointerMain () at checkpointer.c:370
#8 0x00561680 in AuxiliaryProcessMain (argc=2,
argv=0x7fffcfd4bec0) at bootstrap.c:447
I have modified you patch and called InitXLogInsert for CheckpointerProcess
and BgWriterProcess also. After that the
issue is solved and fpw is getting set properly
1 | p1 | p
> (7 rows)
>
Is it a good idea to provide a function or an option which can provide
partitions detail in hierarchical order?
i.e
relname level
p 0
p0 1
p00 2
p01 2
p1 1
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
s / vacrelstats->old_live_tuples
b.qsort((void *)vacrelstats -> qsort((void *) vacrelstats
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
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
> >
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:
> >>>
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
>
tionally, 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
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, 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:
>
On Thu, Nov 15, 2018 at 12:14 PM Dilip Kumar wrote:
>
Updated patch (merged latest code from the zheap main branch [1]).
The main chain is related to removing relfilenode and tablespace id
from the undo record and store reloid.
Earlier, we kept it thinking that we will perform rollback with
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-in
ad, 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
On Wed, Nov 14, 2018 at 3:48 PM Dilip Kumar wrote:
>
> On Wed, Nov 14, 2018 at 2:58 PM Amit Kapila wrote:
> > I think you can keep it with XXX instead of Fixme as there is nothing to
> > fix.
> Changed
> >
> > Both the patches 0003-undo-interface-v4.patch an
artitions that are not pruned.
+ */
prunin/pruning
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
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
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
nner(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
.org/message-id/CAD__OujRZEjE5y3vfmmZmSSr3oYGZSHRxwDwF7kyhBHB2BpW_g%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
walsender process. I have fixed this issue in the patch and also
implemented decode functions for zheap update and multi-insert.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
decode_zops_v4.patch
Description: Binary data
On Sat, Dec 8, 2018 at 7:52 PM Amit Kapila wrote:
> On Tue, Dec 4, 2018 at 3:00 PM Dilip Kumar wrote:
> >
> > On Sat, Dec 1, 2018 at 12:58 PM Amit Kapila
> wrote:
> > >
> > >
> > > 13.
> > > PrepareUndoInsert()
> > > {
> >
On Fri, 30 Nov 2018, 20:42 Dmitry Dolgov <9erthali...@gmail.com wrote:
> > On Mon, Nov 5, 2018 at 1:32 PM Dilip Kumar
> wrote:
> >
> > Updated patch, include defect fix from Kuntal posted on [1].
> >
> > [1]
> https://www.postgresql.org/message-id/CAGz5
On Mon, Nov 26, 2018 at 2:13 PM Amit Kapila wrote:
>
> On Tue, Nov 20, 2018 at 7:37 PM Dilip Kumar wrote:
> > Along with that I have merged latest changes in zheap branch committed
> > by Rafia Sabih for cleaning up the undo buffer information in abort
> > path.
> >
On Sat, Nov 17, 2018 at 5:12 PM Amit Kapila wrote:
>
> On Fri, Nov 16, 2018 at 9:46 AM Dilip Kumar wrote:
> >
> > On Thu, Nov 15, 2018 at 12:14 PM Dilip Kumar wrote:
> > >
> > Updated patch (merged latest code from the zheap main bra
might need few interface changes here and there while
> integrating and testing this with other patches, but the basic idea
> and code look reasonable to me. I have modified the proposed commit
> message in the attached patch, see if that looks fine to you.
>
> To b
On Wed, Jan 9, 2019 at 11:40 AM Dilip Kumar wrote:
> On Tue, Jan 8, 2019 at 2:11 PM Amit Kapila
> wrote:
>
>>
>>
>> 3.
>> + work_txn.urec_next = uur->uur_next;
>> + work_txn.urec_xidepoch = uur->uur_xidepoch;
>> + work_txn.urec_progress =
c4b48639c0e12bf2f70a766b910".
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
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
gres=# FETCH NEXT cur;
a
---
1
(1 row)
postgres=# FETCH 10 cur;
a
---
2
3
4
5
1
2
3
4
5
6
(10 rows)
postgres=# FETCH NEXT cur;
a
---
7
(1 row)
postgres=# CLOSE cur;
CLOSE CURSOR
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
nd fixing the comment
for undo-interface patch. Now, Michael have already moved to new
commitfest with status need review so I guess as of now the status is
correct.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
be worked upon:
a) Get rid of UndoRecInfo
b) Get rid of xid in generic undo code and unify epoch and xid to fxid
c) Get rid of discard lock
d) Move log switch related information from transaction header to new
log switch header
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
, because as part of the other work "Compression
for undo records to consider rmgrid, xid,cid,reloid for each record",
the FullTransactionId, will be present in every UnpackedUndoRecord
(although it will not be stored in every undo record).
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
a file-private struct
> that is known to both the callback and the caller of UndoFetchRecord,
> but not elsewhere.
>
> If we decide we need an iterator within the undo machinery itself,
> then I think it should look like the above, and I think it should
> accept NULL for fou
On Thu, May 9, 2019 at 12:04 PM Dilip Kumar wrote:
>
> On Mon, May 6, 2019 at 5:43 PM Dilip Kumar wrote:
> >
> > Just for tracking, open comments which still needs to be worked on.
> >
> > 1. Avoid special case in UndoRecordIsValid.
> > > Can we instea
On Mon, May 6, 2019 at 5:43 PM Dilip Kumar wrote:
>
> Just for tracking, open comments which still needs to be worked on.
>
> 1. Avoid special case in UndoRecordIsValid.
> > Can we instead eliminate the special case? It seems like the if
> > (log->oldest_data
2019 at 11:14 AM Robert Haas wrote:
> > >
> > > On Tue, Apr 30, 2019 at 2:16 AM Dilip Kumar wrote:
> > > > Like previous version these patch set also applies on:
> > > > https://github.com/EnterpriseDB/zheap/tree/undo
> > > > (b397d96176879ed5b0
on that.
Currently, undo branch[1] contain an older version of the (undo
interface + some fixup). Now, I have merged the latest changes from
the zheap branch[2] to the undo branch[1]
which can be applied on top of the undo storage commit[3]. For
merging those changes, I need to add some changes to the undo log
storage patch as well for handling the multi log transaction. So I
have attached two patches, 1) improvement to undo log storage 2)
complete undo interface patch which include 0006+0007 from undo
branch[1] + new changes on the zheap branch.
[1] https://github.com/EnterpriseDB/zheap/tree/undo
[2] https://github.com/EnterpriseDB/zheap
[3] https://github.com/EnterpriseDB/zheap/tree/undo
(b397d96176879ed5b09cf7322b8d6f2edd8043a5)
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
0001-Enhance-multi-log-handling-in-undo-log.patch
Description: Binary data
0002-Provide-interfaces-to-store-and-fetch-undo-records.patch
Description: Binary data
On Thu, May 2, 2019 at 7:00 PM Robert Haas wrote:
>
> On Thu, May 2, 2019 at 5:32 AM Dilip Kumar wrote:
> > Yeah, at least in this patch it looks silly. Actually, I added that
> > index to make the qsort stable when execute_undo_action sorts them in
> > block order. Bu
essing
so either we can remove this structure completely as you suggested but
undo processing patch has to add that structure or we can just add
comment that why we added this index field.
I am ok with other comments and will work on them.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Apr 30, 2019 at 11:45 AM Dilip Kumar wrote:
The attached patch will provide mechanism for masking the necessary
bits in undo pages for supporting consistency checking for the undo
pages. Ideally we can merge this patch with the main interface patch
but currently I have kept it separate
On Thu, Jul 11, 2019 at 12:38 AM Robert Haas wrote:
>
> On Tue, Jul 9, 2019 at 6:28 AM Dilip Kumar wrote:
> > PFA, updated patch version which includes
> > - One defect fix in undo interface related to undo page compression
> > for handling persistence level
> &
On Sat, Jul 6, 2019 at 8:26 PM Dilip Kumar wrote:
>
> On Thu, Jul 4, 2019 at 5:24 PM Amit Kapila wrote:
> >
> PFA, the latest version of the undo interface and undo processing patches.
>
> Summary of the changes in the patch set
>
> 1. Undo Interface
> - Rebased
ode paths run, so we need a good way to make them not
> rare).
>
In 0003-Add-undo-log-manager
/* If we discarded everything, the slot can be given up. */
+ if (entirely_discarded)
+ free_undo_log_slot(slot);
I have noticed that when the undo log was detached and it was full
then if we discard complete log we release its slot. But, what is
bothering me is should we add that log to the free list? Or I am
missing something?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
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,
> > > un
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
nd 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 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
g 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(>alloc_context,
> > +
> > XidFromFullTransactionId(txid),
> > +
> > size,
> > +
> > _xact_header,
> > +
> > _xact_start,
> > +
> > _xact_start,
> > +
> > );
> > + }
> > + else
> > + {
> > + /* Allocate space for writing the undo record. */
>
> That's basically the same comment as before the if.
Removed
>
>
> > + urecptr = UndoLogAllocate(>alloc_context,
> > + size,
> > +
> > _xact_header, _xact_start,
> > +
> > _xact_start, _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
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,
> > > un
e 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 "update"
Fixed
>
&
be
> a case where this is split across a record? If so, isn't that a bad idea
> anyway?
Yes, as of now, undo record can be splitted at any point even the undo
length can be split acorss 2 pages. I think we can reduce complexity
by making sure undo length doesn't get split acorss pages. But for
handling that while allocating the undo we need to detect this whether
the undo length can get splitted by checking the space in the current
page and the undo record length and based on that we need to allocate
1 extra byte in the undo log. Seems that will add an extra
complexity.
>
>
> > + /* Read buffer if the current buffer is not valid. */
> > + if (!BufferIsValid(buffer))
> > + {
> > + buffer = ReadBufferWithoutRelcache(rnode,
> > UndoLogForkNum,
> > +
> > cur_blk, RBM_NORMAL, NULL,
> > +
> > persistence);
> > +
> > + LockBuffer(buffer, BUFFER_LOCK_SHARE);
> > +
> > + page = BufferGetPage(buffer);
> > + pagedata = (char *) page;
> > + }
> > +
> > + page_offset -= 1;
> > +
> > + /*
> > + * Read current prevlen byte from current block if
> > page_offset hasn't
> > + * reach to undo block header. Otherwise, go to the previous
> > block
> > + * and continue reading from there.
> > + */
> > + if (page_offset >= UndoLogBlockHeaderSize)
> > + {
> > + prevlen[byte_to_read - 1] = pagedata[page_offset];
> > + byte_to_read -= 1;
> > + }
> > + else
> > + {
> > + /*
> > + * Release the current buffer if it is not provide by
> > the caller.
> > + */
> > + if (input_buffer != buffer)
> > + UnlockReleaseBuffer(buffer);
> > +
> > + /*
> > + * Could not read complete prevlen from the current
> > block so go to
> > + * the previous block and start reading from end of
> > the block.
> > + */
> > + cur_blk -= 1;
> > + page_offset = BLCKSZ;
> > +
> > + /*
> > + * Reset buffer so that we can read it again for the
> > previous
> > + * block.
> > + */
> > + buffer = InvalidBuffer;
> > + }
> > + }
>
> I can't help but think that this shouldn't be yet another copy of logic
> for how to read undo pages.
I haven't yet thought but I will try to unify this with ReadUndoBytes.
Actually, I didn't do that already because ReadUndoByte needs a start
pointer where we need to read the given number of bytes but here we
have an end pointer. May be by this logic we can compute the start
pointer but that will look equally complex. I will work on this and
try to figure out something.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
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 know
len =
> > ucontext->urec_payload.urec_tuple_len;
> > +
> > + if (len > 0)
> > + {
> > + /* Insert tuple data. */
> > + if (!InsertUndoBytes((char *)
> > ucontext->urec_tupledata,
> > +
> >len, , endptr,
> > +
> >>already_processed,
> > +
> >>partial_bytes))
> > + return;
> > + }
> > + ucontext->stage = UNDO_PACK_STAGE_UNDO_LENGTH;
> > + }
> > + /* fall through */
> > +
> > + case UNDO_PACK_STAGE_UNDO_LENGTH:
> > + /* Insert undo length. */
> > + if (!InsertUndoBytes((char *) >undo_len,
> > +
> > sizeof(uint16), , endptr,
> > +
> > >already_processed,
> > +
> > >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
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:
> >
> > &
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
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 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 pro
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 inte
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
have read 0003 and 0004 patch and there are few cosmetic comments.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
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
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 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
case what you have given and
I can see the similar improvement with the patch.
With the patch 8832.988, without the patch 10252.701ms (median of three reading)
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
search_path */
+ "where c.relname = 'pgbench_accounts' and o.n is not null "
+ "group by 1, 2 "
+ "order by 1 asc "
I have a question, wouldn't it be sufficient to just group by 1? Are
you expecting multiple pgbench_account tables partitioned by different
strategy under the same schema?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
1 - 100 of 1613 matches
Mail list logo
