On Mon, Nov 4, 2019 at 10:45 AM Amit Kapila wrote:
>
> On Sun, Nov 3, 2019 at 9:49 AM Dilip Kumar wrote:
> >
> > On Fri, Nov 1, 2019 at 2:21 PM Masahiko Sawada
> > wrote:
> > >
> > >
> > > I think that two approaches make parallel vacuum wor
divides the cost limit evenly.
> >
>
> I am also not completely sure which approach is better but I slightly
> lean towards approach (b). I think we need input from some other
> people as well. I will start a separate thread to discuss this and
> see if that helps to get the input from others.
+1
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
pproach (a) with dividing the cost limit)? Currently the approach (b)
> seems better but I'm concerned that it might unnecessarily delay
> vacuum if some indexes are very small or bulk-deletions of indexes
> does almost nothing such as brin.
Are you worried that some of the workers might not have much I/O to do
but still we divide the cost limit equally? If that is the case then
that is the case with the auto vacuum workers also right?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Nov 4, 2019 at 2:11 PM Masahiko Sawada
wrote:
>
> On Mon, 4 Nov 2019 at 17:26, Dilip Kumar wrote:
> >
> > On Mon, Nov 4, 2019 at 1:00 PM Masahiko Sawada
> > wrote:
> > >
> > > On Mon, 4 Nov 2019 at 14:02, Amit Kapila wrote:
> > > >
86 ms (01:30.079)
So your result shows that with "streaming on", performance is
degrading? By any chance did you try to see where is the bottleneck?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ger
> or by hacking the code such that it serializes after every change and
> then if you abort after one change, it should hit this problem.
>
I think you might need to kill the server after all changes are
serialized otherwise normal abort will hit the ReorderBufferAbort and
that will remove your ReorderBufferTXN entry and you will never hit
this case.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
ted the parallel vacuum and non-parallel vacuum with
different ring buffer size)
8 index
ring buffer size 246kb-> non-parallel: 7.6 seconds parallel (2
worker): 3.9 seconds
ring buffer size 256mb-> non-parallel: 6.1 seconds parallel (2
worker): 3.2 seconds
4 index
ring buffer size 246kb -> non-parallel: 4.8 seconds parallel (2
worker): 3.2 seconds
ring buffer size 256mb -> non-parallel: 3.8 seconds parallel (2
worker): 2.6 seconds
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Nov 5, 2019 at 8:49 PM Andres Freund wrote:
>
> Hi,
>
> On November 5, 2019 7:16:41 AM PST, Dilip Kumar wrote:
> >On Tue, Nov 5, 2019 at 2:40 PM Amit Kapila
> >wrote:
> >>
> >> On Mon, Nov 4, 2019 at 11:58 PM Andres Freund
> >wrote:
>
e on the same tablespace. And, the balance can be
checked against its tablespace i/o limit. So If we get such a
mechanism in the future then it seems that it will be easily
expandable to the parallel vacuum, isn't it? Because across workers
also we can track tablespace wise shared balance (if we go with the
shared costing approach for example).
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
the parallel plan then the user can force
and check. But, vacuum parallelism is itself forced by the user so
there is no point in doing it with force_parallel_mode=on. However,
force_parallel_mode=regress is useful for testing the vacuum with an
existing test suit.
>
> Please let me know your thoughts for this patch.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Nov 6, 2019 at 3:50 PM Masahiko Sawada
wrote:
>
> On Wed, 6 Nov 2019 at 18:42, Dilip Kumar wrote:
> >
> > On Wed, Nov 6, 2019 at 2:01 PM Mahendra Singh wrote:
> > >
> > > Hi
> > > I took all attached patches(v32-01 to v32-4) and one Dili
t it gets flushed always.
> 2) Have many open transactions with subtransactions open.
> 3) Attach one of the transaction from gdb and call abort().
Do you need subtransactions for the issue1? It appears that after the
restart if the changes list is empty it will hit the assert. Am I
missing something?
>
> I'm not sure of the fix for this. If I get time I will try to spend
> more time to find out the fix.
> Thoughts?
>
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, 6 Nov 2019, 20:07 Masahiko Sawada,
wrote:
> On Wed, 6 Nov 2019 at 20:29, Dilip Kumar wrote:
> >
> > On Wed, Nov 6, 2019 at 3:50 PM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, 6 Nov 2019 at 18:42, Dilip Kumar
> wrote:
> > > >
On Thu, Nov 7, 2019 at 9:55 AM vignesh C wrote:
>
> On Wed, Nov 6, 2019 at 5:41 PM Dilip Kumar wrote:
> >
> > On Wed, Nov 6, 2019 at 5:20 PM vignesh C wrote:
> > >
> > > Hi,
> > >
> > > I found couple of crashes in reorderbuffer whil
> + rb, rb->spillTxns, rb->spillCount, rb->spillBytes);
>
> Changed the above elog to DEBUG1 as otherwise it was getting printed
> very frequently. I think we can make it DEBUG2 if we want.
Yeah, it should not be WARNING.
>
> 6. There was an extra space in rule
reshold e.g 50% of the local limit
Here I have changed the patch2 as per (b) If local balance reaches to
50% of the local limit and shared balance hit the vacuum cost limit
then go for the delay.
patch 3: Shared costing patch: (delay condition ->
VacuumSharedCostBalance > VacuumCostLimit
On Fri, Nov 8, 2019 at 11:49 AM Amit Kapila wrote:
>
> On Fri, Nov 8, 2019 at 9:39 AM Dilip Kumar wrote:
> >
> > I have done some experiments on this line. I have first produced a
> > case where we can show the problem with the existing shared costing
> > patch (
ring cleanup irrespective of whether bulkdelete is called
or not e.g. gin? If so, along with an amcanparallelcleanup flag, we
need to consider vacrelstats->num_index_scans right? So if
vacrelstats->num_index_scans == 0 then we need to launch parallel
worker for all the indexes who support amcanparallelvacuum and if
vacrelstats->num_index_scans > 0 then only for those who has
amcanparallelcleanup as true.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Nov 11, 2019 at 9:43 AM Dilip Kumar wrote:
>
> On Fri, Nov 8, 2019 at 11:49 AM Amit Kapila wrote:
> >
> > On Fri, Nov 8, 2019 at 9:39 AM Dilip Kumar wrote:
> > >
> > > I have done some experiments on this line. I have first produced a
> > >
On Mon, Nov 11, 2019 at 12:26 PM Masahiko Sawada
wrote:
>
> On Mon, 11 Nov 2019 at 15:06, Dilip Kumar wrote:
> >
> > On Mon, Nov 11, 2019 at 9:57 AM Masahiko Sawada
> > wrote:
> > >
> > > On Fri, 8 Nov 2019 at 18:48, Amit Kapila wrote:
> > >
rs) :
maintenance_work_mem;
Am I missing something?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Nov 11, 2019 at 4:23 PM Amit Kapila wrote:
>
> On Mon, Nov 11, 2019 at 12:59 PM Dilip Kumar wrote:
> >
> > On Mon, Nov 11, 2019 at 9:43 AM Dilip Kumar wrote:
> > >
> > > On Fri, Nov 8, 2019 at 11:49 AM Amit Kapila
> > > wrote:
> > &
On Mon, Nov 11, 2019 at 4:23 PM Amit Kapila wrote:
>
> On Mon, Nov 11, 2019 at 12:59 PM Dilip Kumar wrote:
> >
> > On Mon, Nov 11, 2019 at 9:43 AM Dilip Kumar wrote:
> > >
> > > On Fri, Nov 8, 2019 at 11:49 AM Amit Kapila
> > > wrote:
> > &
; parallel (Indexes nbtree, hash, gin, gist, spgist, bloom will set this
> flag)
> VACUUM_OPTION_PARALLEL_COND_CLEANUP 3 # vacuumcleanup can be done in
> parallel if bulkdelete is not performed (Indexes nbtree, brin, hash,
> gin, gist, spgist, bloom will set this flag)
> VACUUM_OPTION_PARALLEL_CLEANUP 4 # vacuumcleanup can be done in
> parallel even if bulkdelete is already performed (Indexes gin, brin,
> and bloom will set this flag)
>
> Does something like this make sense?
Yeah, something like that seems better to me.
> If we all agree on this, then I
> think we can summarize the part of the discussion related to this API
> and get feedback from a broader audience.
Make sense.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Nov 12, 2019 at 10:47 AM Dilip Kumar wrote:
>
> On Mon, Nov 11, 2019 at 4:23 PM Amit Kapila wrote:
> >
> > On Mon, Nov 11, 2019 at 12:59 PM Dilip Kumar wrote:
> > >
> > > On Mon, Nov 11, 2019 at 9:43 AM Dilip Kumar wrote:
> > > >
>
On Tue, Nov 12, 2019 at 4:04 PM Masahiko Sawada
wrote:
>
> On Mon, 11 Nov 2019 at 17:57, Dilip Kumar wrote:
> >
> > On Tue, Oct 29, 2019 at 12:37 PM Masahiko Sawada
> > wrote:
> > > I realized that v31-0006 patch doesn't work fine so I've attache
; > > > for cleanup.
> > > >
> > > > VACUUM_OPTION_PARALLEL_NO_BULKDEL 1 << 0
> > > > VACUUM_OPTION_PARALLEL_BULKDEL 1 << 1
> > > > VACUUM_OPTION_PARALLEL_NO_CLEANUP 1 << 2
> > > > VACUUM_OPTION_PARALLEL_COND_CLEANUP 1 << 3
> > > > VACUUM_OPTION_PARALLEL_CLEANUP 1 << 4
> > > >
> > >
> > > This also looks reasonable, but if there is an index that doesn't want
> > > to support a parallel vacuum, it needs to set multiple flags.
> >
> > Right. It would be better to use uint16 as two uint8. I mean that if
> > first 8 bits are 0 it means VACUUM_OPTION_PARALLEL_NO_BULKDEL and if
> > next 8 bits are 0 means VACUUM_OPTION_PARALLEL_NO_CLEANUP. Other flags
> > could be followings:
> >
> > VACUUM_OPTION_PARALLEL_BULKDEL 0x0001
> > VACUUM_OPTION_PARALLEL_COND_CLEANUP 0x0100
> > VACUUM_OPTION_PARALLEL_CLEANUP 0x0200
> >
>
> Hmm, I think we should define these flags in the most simple way.
> Your previous proposal sounds okay to me.
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Nov 12, 2019 at 5:31 PM Masahiko Sawada
wrote:
>
> On Tue, 12 Nov 2019 at 20:29, Dilip Kumar wrote:
> >
> > On Tue, Nov 12, 2019 at 4:04 PM Masahiko Sawada
> > wrote:
> > >
> > > On Mon, 11 Nov 2019 at 17:57, Dilip Kumar wrote:
> > >
On Wed, Nov 13, 2019 at 9:12 AM Dilip Kumar wrote:
>
> On Tue, Nov 12, 2019 at 5:31 PM Masahiko Sawada
> wrote:
> >
> > On Tue, 12 Nov 2019 at 20:29, Dilip Kumar wrote:
> > >
> > > On Tue, Nov 12, 2019 at 4:04 PM Masahiko Sawada
> > > wrote:
>
ssed to expose this information via two variables but the
> above seems like a better idea to all the people involved.
>
> Any suggestions? Anyone thinks this is not the right way to expose
> this information or there is no need to expose this information or
> they have a better idea for this?
>
> Sawada-San, Dilip, feel free to correct me.
Looks fine to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Wed, Nov 13, 2019 at 11:39 AM Masahiko Sawada
wrote:
>
> On Wed, 13 Nov 2019 at 12:43, Dilip Kumar wrote:
> >
> > On Tue, Nov 12, 2019 at 5:31 PM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, 12 Nov 2019 at 20:29, Dilip Kumar wrote:
> > >
On Wed, Nov 13, 2019 at 5:55 PM Amit Kapila wrote:
>
> On Thu, Oct 3, 2019 at 1:18 PM Dilip Kumar wrote:
> >
>
> As mentioned by me a few days back that the first patch in this series
> is ready to go [1] (I am hoping Tomas will pick it up), so I have
> started the
On Thu, Nov 14, 2019 at 12:10 PM Amit Kapila wrote:
>
> On Thu, Nov 14, 2019 at 9:37 AM Dilip Kumar wrote:
> >
> > On Wed, Nov 13, 2019 at 5:55 PM Amit Kapila wrote:
> > >
> > > On Thu, Oct 3, 2019 at 1:18 PM Dilip Kumar wrote:
> > > >
> &g
On Thu, Nov 14, 2019 at 5:02 PM Mahendra Singh wrote:
>
> On Mon, 11 Nov 2019 at 17:56, Amit Kapila wrote:
> >
> > On Mon, Nov 11, 2019 at 5:14 PM Dilip Kumar wrote:
> > >
> > > On Mon, Nov 11, 2019 at 4:23 PM Amit Kapila
> > > wrote:
> > &
On Fri, Nov 15, 2019 at 3:50 PM Amit Kapila wrote:
>
> On Thu, Nov 14, 2019 at 3:40 PM Dilip Kumar wrote:
> >
> >
> > Apart from this, I have another question in
> > 0003-Issue-individual-invalidations-with-wal_level-logical.patch
> >
> > @@ -543,6 +
On Fri, Nov 15, 2019 at 4:19 PM Amit Kapila wrote:
>
> On Fri, Nov 15, 2019 at 4:01 PM Dilip Kumar wrote:
> >
> > On Fri, Nov 15, 2019 at 3:50 PM Amit Kapila wrote:
> > >
> > >
> > > Few other comments on this patch:
> > &
On Tue, Nov 19, 2019 at 5:23 PM Amit Kapila wrote:
>
> On Mon, Nov 18, 2019 at 5:02 PM Dilip Kumar wrote:
> >
> > On Fri, Nov 15, 2019 at 4:19 PM Amit Kapila wrote:
> > >
> > > On Fri, Nov 15, 2019 at 4:01 PM Dilip Kumar wrote:
> > > >
>
On Wed, Nov 20, 2019 at 8:22 PM Dilip Kumar wrote:
>
> On Wed, Nov 20, 2019 at 11:15 AM Dilip Kumar wrote:
> >
> > On Tue, Nov 19, 2019 at 5:23 PM Amit Kapila wrote:
> > >
> > > On Mon, Nov 18, 2019 at 5:02 PM Dilip Kumar wrote:
> > > >
>
needs to access it.
> >
>
> We can probably copy the stats in local memory instead of pointing it
> to dsm after bulk-deletion, but I think that would unnecessary
> overhead and doesn't sound like a good idea.
I agree that it will be unnecessary overhead.
>
> > > BTW, what kind of API change you have in mind for
> > > the approach you are suggesting?
> >
> > I was thinking to add a new API, say LaunchParallelNWorkers(pcxt, n),
> > where n is the number of workers the caller wants to launch and should
> > be lower than the value in the parallel context.
> >
>
> For that won't you need to duplicate most of the code of
> LaunchParallelWorkers or maybe move the entire code in
> LaunchParallelNWorkers and then LaunchParallelWorkers can also call
> it. Another idea could be to just extend the existing API
> LaunchParallelWorkers to take input parameter as the number of
> workers, do you see any problem with that or is there a reason you
> prefer to write a new API for this?
I think we can pass an extra parameter to LaunchParallelWorkers
therein we can try to launch min(pcxt->nworkers, n). Or we can put an
assert (n <= pcxt->nworkers).
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
al = 0;
+ }
+
+ if (pg_atomic_compare_exchange_u32(VacuumSharedCostBalance,
+&shared_balance,
+new_balance))
+ {
+ /* Updated successfully, break */
+ break;
+ }
While looking at the shared costing delay part, I have noticed that
while checking the delay condition, we are considering local_balance
which is VacuumCostBalanceLocal + VacuumCostBalance, but while
computing the new balance we only reduce shared balance by
VacuumCostBalanceLocal, I think it should be reduced with
local_balance? I see that later we are adding VacuumCostBalance to
the VacuumCostBalanceLocal so we are not loosing accounting for this
balance. But, I feel it is not right that we compare based on one
value and operate based on other. I think we can immediately set
VacuumCostBalanceLocal += VacuumCostBalance before checking the
condition.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Nov 21, 2019 at 10:46 AM Dilip Kumar wrote:
>
> On Wed, Nov 20, 2019 at 11:01 AM Masahiko Sawada
> wrote:
> >
> > On Mon, 18 Nov 2019 at 15:38, Masahiko Sawada
> > wrote:
> > >
> > > On Mon, 18 Nov 2019 at 15:34, Amit Kapila wrote:
>
On Thu, 21 Nov 2019, 13:52 Masahiko Sawada,
wrote:
> On Thu, 21 Nov 2019 at 14:16, Dilip Kumar wrote:
> >
> > On Wed, Nov 20, 2019 at 11:01 AM Masahiko Sawada
> > wrote:
> > >
> > > On Mon, 18 Nov 2019 at 15:38, Masahiko Sawada
> > > wrote:
&g
On Thu, 21 Nov 2019, 14:15 Masahiko Sawada,
wrote:
> On Thu, 21 Nov 2019 at 14:32, Dilip Kumar wrote:
> >
> > On Thu, Nov 21, 2019 at 10:46 AM Dilip Kumar
> wrote:
> > >
> > > On Wed, Nov 20, 2019 at 11:01 AM Masahiko Sawada
> > > wrote:
> >
In logical decoding, while sending the changes to the output plugin we
need to arrange them in the LSN order. But, if there is only one
transaction which is a very common case then we can avoid building the
binary heap. A small patch is attached for the same.
--
Regards,
Dilip Kumar
lar effect.
> If we have 0 missed indexes - parallel vacuum will run as in current
> implementation, with leader participation.
+1
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
nother option, but it might not be a good idea
> for partial indexes.
>
> > That is, we cannot do parallel vacuum on the table smaller than that
> > value.
> >
>
> Yeah, that makes sense, but I feel if we can directly target index
> scan size that may be a better option. If we can't use
> min_parallel_index_scan_size, then we can consider this.
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Dec 5, 2019 at 12:21 AM Masahiko Sawada
wrote:
>
> On Wed, 4 Dec 2019 at 04:57, Dilip Kumar wrote:
> >
> > On Wed, Dec 4, 2019 at 9:12 AM Amit Kapila wrote:
> > >
> > > On Wed, Dec 4, 2019 at 1:58 AM Masahiko Sawada
> > > wrote:
> > &
; + * For updating the index statistics, since any updates are not allowed
> during
> + * parallel mode we update the index statistics after exited from the
> parallel
>
> The first of these sentences ("Note that all...") is not very clear to
> me, and seems like it may amount to a statement that the leader
> doesn't try to destroy the parallel context too early, but since I
> don't understand it, maybe that's not what it is saying. The second
> sentence needs exited -> exiting, and maybe some more work on the
> grammar, too.
>
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Dec 6, 2019 at 12:55 AM Mahendra Singh wrote:
>
> On Thu, 5 Dec 2019 at 19:54, Robert Haas wrote:
> >
> > [ Please trim excess quoted material from your replies. ]
> >
> > On Thu, Dec 5, 2019 at 12:27 AM Dilip Kumar wrote:
> > > I agree that t
On Mon, Dec 2, 2019 at 2:01 PM Dilip Kumar wrote:
>
> On Sun, Dec 1, 2019 at 7:58 AM Michael Paquier wrote:
> >
> > On Fri, Nov 22, 2019 at 01:18:11PM +0530, Dilip Kumar wrote:
> > > I have rebased the patch on the latest head and also fix the issue of
> > &g
On Tue, Dec 10, 2019 at 9:52 AM Amit Kapila wrote:
>
> On Mon, Dec 2, 2019 at 2:02 PM Dilip Kumar wrote:
> >
> > On Sun, Dec 1, 2019 at 7:58 AM Michael Paquier wrote:
> > >
> > > On Fri, Nov 22, 2019 at 01:18:11PM +0530, Dilip Kumar wrote:
> > > >
new version with a slightly modified commit message.
Your changes look fine to me. Thanks!
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
because
that depends upon whether it gets the CLogControlLock or not.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
v1-0001-Change-wrong-assert-while-group-updating-xid-stat.patch
Description: Binary data
On Wed, Dec 11, 2019 at 5:22 PM Amit Kapila wrote:
>
> On Mon, Dec 9, 2019 at 1:27 PM Dilip Kumar wrote:
> >
> > I have review the patch set and here are few comments/questions
> >
> > 1.
> > +static void
> > +pg_decode_stream_change(LogicalDecodin
On Mon, Nov 25, 2019 at 9:22 AM Dilip Kumar wrote:
>
> In logical decoding, while sending the changes to the output plugin we
> need to arrange them in the LSN order. But, if there is only one
> transaction which is a very common case then we can avoid building the
> binary heap.
difficult to enhance or
> provide the new APIs to support a parallel vacuum if we come across
> such a usage. I think we should just modify the comments atop
> VACUUM_OPTION_NO_PARALLEL to mention this. I think this should be
> good enough for the first version of parallel vacuum considering we
> are able to support a parallel vacuum for all in-core indexes.
>
> Thoughts?
+1
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Sat, Dec 28, 2019 at 9:33 PM Tomas Vondra
wrote:
>
> On Tue, Dec 10, 2019 at 10:23:19AM +0530, Dilip Kumar wrote:
> >On Tue, Dec 10, 2019 at 9:52 AM Amit Kapila wrote:
> >>
> >> On Mon, Dec 2, 2019 at 2:02 PM Dilip Kumar wrote:
> >> >
> >&g
On Mon, Dec 30, 2019 at 3:43 PM Amit Kapila wrote:
>
> On Sun, Dec 29, 2019 at 1:34 PM Dilip Kumar wrote:
> >
> > I have observed some more issues
> >
> > 1. Currently, In ReorderBufferCommit, it is always expected that
> > whenever we get REORDER_BUFFER_C
d as mentioned earlier that we
> started with the approach of separate booleans, but later found that
> this is a better way as it was easier to set and check the different
> parallel options for a parallel vacuum. I think we can go back to
> the individual booleans if we wa
when we allow
> supporting multiple workers for an index, we might need another
> variable unless we can allow it for all types of indexes. This was my
> point that having multiple variables for the purpose of a parallel
> vacuum (for indexes) doesn't sound like a better approach than having
> a single uint8 variable.
>
> > If we don't stick to have only
> > booleans the having a ternary value for cleanup might be
> > understandable but I'm not sure it's better to have it for only vacuum
> > purpose.
> >
>
> If we want to keep the possibility of extending it for other purposes,
> then we can probably rename it to amoptions or something like that.
> What do you think?
I think it makes more sense to just keep it for the purpose of
enabling/disabling parallelism in different phases. I am not sure
that adding more options (which are not related to enabling
parallelism in vacuum phases) to the same variable will make sense.
So I think the current name is good for its purpose.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
s the same can be served by setting vacuum_cost_delay = 0 which is a valid
> argument, but OTOH, providing an option to the user which can make his life
> easier is not a bad idea either.
I agree that there is already an option to run it without cost delay
but there is no harm
On Sat, Jan 4, 2020 at 10:00 AM Amit Kapila wrote:
>
> On Sun, Dec 29, 2019 at 1:34 PM Dilip Kumar wrote:
> > On Sat, Dec 28, 2019 at 9:33 PM Tomas Vondra
> > wrote:
> > +static void
> > +set_schema_sent_in_streamed_txn(RelationSyncEntry
On Sat, Jan 4, 2020 at 4:07 PM Amit Kapila wrote:
>
> On Mon, Dec 30, 2019 at 3:11 PM Dilip Kumar wrote:
> >
> > On Thu, Dec 12, 2019 at 9:44 AM Dilip Kumar wrote:
> > >
> > Yesterday, Tomas has posted the latest version of the patch set which
> &g
> replicating from a regular table on publisher node into a partitioned
> table of the same name on subscriber node (as the earlier patches
> did), because 0001 doesn't implement tuple routing support that would
> be needed to apply such changes.
>
> Attached updated patches.
>
I am planning to review this patch. Currently, it is not applying on
the head so can you rebase it?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Jan 6, 2020 at 2:11 PM Amit Kapila wrote:
>
> On Mon, Jan 6, 2020 at 9:21 AM Dilip Kumar wrote:
> >
> > On Sat, Jan 4, 2020 at 4:07 PM Amit Kapila wrote:
> > >
> > >
> > > It is better to merge it with the main patch for
> > > &quo
On Mon, Jan 6, 2020 at 4:36 PM Amit Kapila wrote:
>
> On Mon, Jan 6, 2020 at 3:56 PM Dilip Kumar wrote:
> >
> > On Mon, Jan 6, 2020 at 2:11 PM Amit Kapila wrote:
> > >
> > > 3.
> > > +static void
> > > +ReorderBu
N is for the perfectly correlated case (csquared=1) */
+ pages_fetchedMIN = ceil(indexSelectivity * (double) baserel->pages);
+
+ pages_fetched = pages_fetchedMAX +
indexCorrelation*indexCorrelation*(pages_fetchedMIN -
pages_fetchedMAX);
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Mon, Jan 6, 2020 at 4:44 PM Dilip Kumar wrote:
>
> On Mon, Jan 6, 2020 at 4:36 PM Amit Kapila wrote:
> >
> > On Mon, Jan 6, 2020 at 3:56 PM Dilip Kumar wrote:
> > >
> > > On Mon, Jan 6, 2020 at 2:11 PM Amit Kapila
> > > w
On Wed, 8 Jan 2020 at 5:28 PM, Heikki Linnakangas wrote:
> On 25/11/2019 05:52, Dilip Kumar wrote:
> > In logical decoding, while sending the changes to the output plugin we
> > need to arrange them in the LSN order. But, if there is only one
> > transaction which is a ver
On Thu, Jan 9, 2020 at 9:35 AM Amit Kapila wrote:
>
> On Wed, Jan 8, 2020 at 1:12 PM Dilip Kumar wrote:
> >
> > I have observed one more design issue.
> >
>
> Good observation.
>
> > The problem is that when we
> > get a toasted chunks we remember
On Thu, Jan 9, 2020 at 12:09 PM Amit Kapila wrote:
>
> On Thu, Jan 9, 2020 at 10:30 AM Dilip Kumar wrote:
> >
> > On Thu, Jan 9, 2020 at 9:35 AM Amit Kapila wrote:
> > >
> > > On Wed, Jan 8, 2020 at 1:12 PM Dilip Kumar wrote:
> > > &g
rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL ||
- rel->rd_rel->relreplident == REPLICA_IDENTITY_INDEX);
+ rel->rd_rel->relreplident == REPLICA_IDENTITY_INDEX ||
+ rel->rd_rel->relreplident == REPLICA_IDENTITY_NOTHING);
/* use Oid as relation identifier */
pq_sendint32(out, RelationGetRelid(rel));
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, 9 Jan 2020 at 10:43 PM, Andres Freund wrote:
> Hi,
>
> On 2020-01-09 13:17:59 +0530, Dilip Kumar wrote:
> > I am able to reproduce the failure, I think the assert in the
> > 'logicalrep_write_insert' is not correct. IMHO even if the replica
> > id
On Sat, Jan 4, 2020 at 4:07 PM Amit Kapila wrote:
>
> On Mon, Dec 30, 2019 at 3:11 PM Dilip Kumar wrote:
> >
> > On Thu, Dec 12, 2019 at 9:44 AM Dilip Kumar wrote:
> > > > 0002-Issue-individual-in
On Mon, Dec 30, 2019 at 3:43 PM Amit Kapila wrote:
>
> On Sun, Dec 29, 2019 at 1:34 PM Dilip Kumar wrote:
> >
> > I have observed some more issues
> >
> > 1. Currently, In ReorderBufferCommit, it is always expected that
> > whenever we get REORDER_BUFFER_C
On Fri, Jan 10, 2020 at 10:31 AM Michael Paquier wrote:
>
> On Fri, Jan 10, 2020 at 07:30:34AM +0530, Dilip Kumar wrote:
> > On Thu, 9 Jan 2020 at 10:43 PM, Andres Freund wrote:
> >> There's not much point in having this assert, right? Given that it
> >> cover
and empty leaf pages. They are
> + * used for deleting all the empty pages.
> */
> After dot, there should be 2 spaces. Earlier, there was 2 spaces.
>
> Other than that patch looks fine to me.
>
Thanks for the comment. Amit has already taken care of this before pushing it.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
>
> > >
> > > Yeah, we can improve the situation here. I think we don't need to
> > > change the value of params->nworkers at first place if allow
> > > lazy_scan_heap to take care of this. Also, I think we shouldn't
> > > display warning unless the user has explicitly asked for parallel
> > > option. See the fix in the attached patch.
> >
> > Agreed. But with the updated patch the PARALLEL option without the
> > parallel degree doesn't display warning because params->nworkers = 0
> > in that case. So how about restoring params->nworkers at the end of
> > vacuum_rel()?
> >
>
> I had also thought on those lines, but I was not entirely sure about
> this resetting of workers. Today, again thinking about it, it seems
> the idea Mahendra is suggesting that is giving an error if the
> parallel degree is not specified seems reasonable to me. This means
> Vacuum (parallel), Vacuum (parallel) , etc. will give an
> error "parallel degree must be specified". This idea has merit as now
> we are supporting a parallel vacuum by default, so a 'parallel' option
> without a parallel degree doesn't have any meaning. If we do that,
> then we don't need to do anything additional about the handling of
> temp tables (other than what patch is already doing) as well. What do
> you think?
>
This idea make sense to me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Jan 9, 2020 at 12:09 PM Amit Kapila wrote:
>
> On Thu, Jan 9, 2020 at 10:30 AM Dilip Kumar wrote:
> >
> > On Thu, Jan 9, 2020 at 9:35 AM Amit Kapila wrote:
> > >
> > > On Wed, Jan 8, 2020 at 1:12 PM Dilip Kumar wrote:
> > > &g
on a transaction when we decode its commit or abort
+ * record, but we never see those records for crashed transactions. To
+ * ensure cleanup of these transactions, set final_lsn to that of their
+ * last change; this causes ReorderBufferRestoreCleanup to do the right
+ * thing. Final_lsn would have been set with commit_lsn earlier when we
+ * decode it commit, no need to update in that case
+ */
+ if (txn->final_lsn < change->lsn)
+ txn->final_lsn = change->lsn;
/decode it commit,/decode its commit,
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Jan 17, 2020 at 7:42 AM vignesh C wrote:
>
> On Thu, Jan 16, 2020 at 9:17 AM Dilip Kumar wrote:
> >
> > One minor comment. Otherwise, the patch looks fine to me.
> > + /*
> > + * We set final_lsn on a transaction when we decode its commit or abort
> >
s->lvshared->cost_balance and then
uninitialize if nworkers_launched is 0.
I am not sure why do we need to initialize VacuumSharedCostBalance
here? just to perform pg_atomic_write_u32(VacuumSharedCostBalance,
VacuumCostBalance);?
I think we can initialize it only if nworkers_launched > 0 then we can
get rid of the else branch completely.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Fri, Jan 17, 2020 at 9:36 AM Dilip Kumar wrote:
>
> On Thu, Jan 16, 2020 at 5:34 PM Amit Kapila wrote:
> >
> > On Thu, Jan 16, 2020 at 4:46 PM Masahiko Sawada
> > wrote:
> > >
> > > Right. Most indexes (all?) of tables that are used in t
On Fri, Jan 17, 2020 at 10:44 AM Amit Kapila wrote:
>
> On Fri, Jan 17, 2020 at 9:36 AM Dilip Kumar wrote:
> >
> > I have few small comments.
> >
> > 1.
> > logical streaming for large in-progress transactions+
> > + /* Can't perform vacuum
On Fri, Jan 17, 2020 at 11:34 AM Amit Kapila wrote:
>
> On Fri, Jan 17, 2020 at 11:00 AM Dilip Kumar wrote:
> >
> > On Fri, Jan 17, 2020 at 10:44 AM Amit Kapila
> > wrote:
> > >
> > > On Fri, Jan 17, 2020 at 9:36 AM Dilip Kumar wrote:
On Fri, Jan 17, 2020 at 11:39 AM Dilip Kumar wrote:
I have performed cost delay testing on the latest test(I have used
same script as attahced in [1] and [2].
vacuum_cost_delay = 10
vacuum_cost_limit = 2000
Observation: As we have concluded earlier, the delay time is in sync
with the I/O
to me except, we better use parentheses for the
variable passed in macro.
+#define MAXDEADTUPLES(max_size) \
+ ((max_size - offsetof(LVDeadTuples, itemptrs)) / sizeof(ItemPointerData))
change to -> (((max_size) - offsetof(LVDeadTuples, itemptrs)) /
sizeof(ItemPointerData))
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
.org/message-id/CAFiTN-t8PmKA1X4jEqKmkvs0ggWpy0APWpPuaJwpx2YpfAf97w%40mail.gmail.com
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Tue, Jan 28, 2020 at 11:28 AM Amit Kapila wrote:
>
> On Wed, Jan 22, 2020 at 10:30 AM Dilip Kumar wrote:
> >
> > On Tue, Jan 14, 2020 at 10:44 AM Amit Kapila
> > wrote:
> > >
> > >
> > > Hmm, I think this can turn out to be inefficient b
On Tue, Jan 28, 2020 at 11:43 AM Amit Kapila wrote:
>
> On Tue, Jan 28, 2020 at 11:34 AM Dilip Kumar wrote:
> >
> > On Tue, Jan 28, 2020 at 11:28 AM Amit Kapila
> > wrote:
> > >
> > > On Wed, Jan 22, 2020 at 10:30 AM Dilip Kumar
> > > wrot
On Tue, Jan 28, 2020 at 1:30 PM Amit Kapila wrote:
>
> On Tue, Jan 28, 2020 at 11:58 AM Dilip Kumar wrote:
> >
> > On Tue, Jan 28, 2020 at 11:43 AM Amit Kapila
> > wrote:
> > >
> > > > > It seems to me that we need to add all of this new hand
atch is fixups from me to bring that code into line with
> changes to the lower level patches. Future versions will be squashed
> and tidied up; still working 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 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
have that processing
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 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
M 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 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 == Inva
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
er such declarations.
Pending items to 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
Ent
access the statistics of the table if a user has privilege on
its parent table?
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
501 - 600 of 1819 matches
Mail list logo
