Thanks a lot Tom.
On Tue, Jul 13, 2021 at 2:37 AM Tom Lane wrote:
>
> Amul Sul writes:
> > [ v5_Add-RelationGetSmgr-inline-function.patch ]
>
> Pushed with minor cosmetic adjustments.
>
> RelationCopyStorage() kind of gives me the willies.
> It's not really an smgr-level function, but we call it
Amul Sul writes:
> [ v5_Add-RelationGetSmgr-inline-function.patch ]
Pushed with minor cosmetic adjustments.
RelationCopyStorage() kind of gives me the willies.
It's not really an smgr-level function, but we call it
everywhere with smgr pointers that belong to relcache entries:
/* copy m
On Fri, Jul 9, 2021 at 7:30 PM Alvaro Herrera wrote:
>
> On 2021-Jul-09, Amul Sul wrote:
>
> > > On Tue, Jul 6, 2021 at 11:06 PM Tom Lane wrote:
>
> > > > The point of the static-inline function idea was to be cheap enough
> > > > that it isn't worth worrying about this sort of risky optimization
On 2021-Jul-09, Amul Sul wrote:
> > On Tue, Jul 6, 2021 at 11:06 PM Tom Lane wrote:
> > > The point of the static-inline function idea was to be cheap enough
> > > that it isn't worth worrying about this sort of risky optimization.
> > > Given that an smgr function is sure to involve some kernel
On Wed, Jul 7, 2021 at 9:44 AM Amul Sul wrote:
>
> On Tue, Jul 6, 2021 at 11:06 PM Tom Lane wrote:
> >
> > Amul Sul writes:
> > > On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi
> > > wrote:
> > >> I don't mind RelationGetSmgr(index)->smgr_rnode alone or
> > >> &variable->member alone and the
On Tue, Jul 6, 2021 at 11:06 PM Tom Lane wrote:
>
> Amul Sul writes:
> > On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi
> > wrote:
> >> I don't mind RelationGetSmgr(index)->smgr_rnode alone or
> >> &variable->member alone and there's not the previous call to
> >> RelationGetSmgr just above. H
Amul Sul writes:
> On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi
> wrote:
>> I don't mind RelationGetSmgr(index)->smgr_rnode alone or
>> &variable->member alone and there's not the previous call to
>> RelationGetSmgr just above. How about using a temporary variable?
>>
>> SMgrRelation srel =
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
I looked through the patch. Looks good to me.
CFbot tests are passi
On Tue, Apr 20, 2021 at 6:59 AM Kyotaro Horiguchi
wrote:
>
> At Mon, 19 Apr 2021 16:27:25 +0530, Amul Sul wrote in
> > On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi
> > wrote:
> > > + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM,
> > > BLOOM_METAPAGE_BLKNO,
> > >
At Mon, 19 Apr 2021 16:27:25 +0530, Amul Sul wrote in
> On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi
> wrote:
> > + smgrwrite(RelationGetSmgr(index), INIT_FORKNUM,
> > BLOOM_METAPAGE_BLKNO,
> > (char *) metapage, true);
> > - log_newpage(&index->rd_smg
At Mon, 19 Apr 2021 09:19:36 -0400, Tom Lane wrote in
> Michael Paquier writes:
> > On Mon, Apr 19, 2021 at 05:35:52PM +0900, Kyotaro Horiguchi wrote:
> >> Isn't this a kind of open item?
>
> > This does not qualify as an open item because it is not an actual bug
> > IMO, neither is it a defect
Michael Paquier writes:
> On Mon, Apr 19, 2021 at 05:35:52PM +0900, Kyotaro Horiguchi wrote:
>> Isn't this a kind of open item?
> This does not qualify as an open item because it is not an actual bug
> IMO, neither is it a defect of the existing code, so it seems
> appropriate to me to not list i
On Mon, Apr 19, 2021 at 05:35:52PM +0900, Kyotaro Horiguchi wrote:
> Isn't this a kind of open item?
This does not qualify as an open item because it is not an actual bug
IMO, neither is it a defect of the existing code, so it seems
appropriate to me to not list it.
--
Michael
signature.asc
Desc
On Mon, Apr 19, 2021 at 04:27:25PM +0530, Amul Sul wrote:
> On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi
> wrote:
> >
> > At Mon, 19 Apr 2021 12:56:18 +0530, Amul Sul wrote in
> > > On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier
> > > wrote:
> > > >
> > > > On Fri, Apr 09, 2021 at 06:45
On Mon, Apr 19, 2021 at 2:05 PM Kyotaro Horiguchi
wrote:
>
> At Mon, 19 Apr 2021 12:56:18 +0530, Amul Sul wrote in
> > On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier
> > wrote:
> > >
> > > On Fri, Apr 09, 2021 at 06:45:45PM -0400, Alvaro Herrera wrote:
> > > > We forgot this patch earlier in
At Mon, 19 Apr 2021 12:56:18 +0530, Amul Sul wrote in
> On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier wrote:
> >
> > On Fri, Apr 09, 2021 at 06:45:45PM -0400, Alvaro Herrera wrote:
> > > We forgot this patch earlier in the commitfest. Do people think we
> > > should still get it in on this c
On Mon, Apr 19, 2021 at 12:25 PM Michael Paquier wrote:
>
> On Fri, Apr 09, 2021 at 06:45:45PM -0400, Alvaro Herrera wrote:
> > We forgot this patch earlier in the commitfest. Do people think we
> > should still get it in on this cycle? I'm +1 on that, since it's a
> > safety feature poised to p
On Fri, Apr 09, 2021 at 06:45:45PM -0400, Alvaro Herrera wrote:
> We forgot this patch earlier in the commitfest. Do people think we
> should still get it in on this cycle? I'm +1 on that, since it's a
> safety feature poised to prevent more bugs than it's likely to
> introduce.
No objections fr
On 2021-Mar-25, Amul Sul wrote:
> Ok, in the attached patch, I have added the inline function to rel.h, and for
> that, I end up including smgr.h to rel.h. I tried to replace all rel->rd_smgr
> by RelationGetSmgr() function and removed the RelationOpenSmgr() call from
> the nearby to it which I do
On Thu, Mar 25, 2021 at 12:10 PM Kyotaro Horiguchi
wrote:
>
> Sorry for the bug.
>
> At Thu, 25 Mar 2021 01:50:29 -0400, Tom Lane wrote in
> > Amul Sul writes:
> > > On Wed, Mar 24, 2021 at 8:09 PM Tom Lane wrote:
> > >> static inline struct SMgrRelationData *
> > >> RelationGetSmgr(Relation re
Sorry for the bug.
At Thu, 25 Mar 2021 01:50:29 -0400, Tom Lane wrote in
> Amul Sul writes:
> > On Wed, Mar 24, 2021 at 8:09 PM Tom Lane wrote:
> >> static inline struct SMgrRelationData *
> >> RelationGetSmgr(Relation rel)
> >> {
> >> if (unlikely(rel->rd_smgr == NULL))
> >> Relat
Amul Sul writes:
> On Wed, Mar 24, 2021 at 8:09 PM Tom Lane wrote:
>> static inline struct SMgrRelationData *
>> RelationGetSmgr(Relation rel)
>> {
>> if (unlikely(rel->rd_smgr == NULL))
>> RelationOpenSmgr(rel);
>> return rel->rd_smgr;
>> }
> A quick question: Can't it be a macr
On Wed, Mar 24, 2021 at 8:09 PM Tom Lane wrote:
>
> Amul Sul writes:
> > On Tue, Mar 23, 2021 at 8:59 PM Tom Lane wrote:
> >> On closer inspection, I believe the true culprit is c6b92041d,
> >> which did this:
> >> - heap_sync(state->rs_new_rel);
> >> + smgrimmedsync(
Amul Sul writes:
> On Tue, Mar 23, 2021 at 8:59 PM Tom Lane wrote:
>> On closer inspection, I believe the true culprit is c6b92041d,
>> which did this:
>> - heap_sync(state->rs_new_rel);
>> + smgrimmedsync(state->rs_new_rel->rd_smgr, MAIN_FORKNUM);
>> heap_sync was car
On Tue, Mar 23, 2021 at 8:59 PM Tom Lane wrote:
>
> I wrote:
> > Michael Paquier writes:
> >> One bisect later, the winner is:
> >> commit: 3d351d916b20534f973eda760cde17d96545d4c4
> >> author: Tom Lane
> >> date: Sun, 30 Aug 2020 12:21:51 -0400
> >> Redefine pg_class.reltuples to be -1 before t
I wrote:
> Michael Paquier writes:
>> One bisect later, the winner is:
>> commit: 3d351d916b20534f973eda760cde17d96545d4c4
>> author: Tom Lane
>> date: Sun, 30 Aug 2020 12:21:51 -0400
>> Redefine pg_class.reltuples to be -1 before the first VACUUM or ANALYZE.
> I think that's an artifact. That
Michael Paquier writes:
> On Tue, Mar 23, 2021 at 04:12:01PM +0900, Michael Paquier wrote:
>> It takes some time to initialize a cluster under CLOBBER_CACHE_ALWAYS,
>> but the test is quick enough to reproduce. It would be good to bisect
>> the origin point here as a first step.
> One bisect lat
On Tue, Mar 23, 2021 at 04:12:01PM +0900, Michael Paquier wrote:
> It takes some time to initialize a cluster under CLOBBER_CACHE_ALWAYS,
> but the test is quick enough to reproduce. It would be good to bisect
> the origin point here as a first step.
One bisect later, the winner is:
commit: 3d351
On Tue, Mar 23, 2021 at 10:52:09AM +0530, Neha Sharma wrote:
> Sure, will give a regression run with CCA enabled.
I can confirm the regression between 13 and HEAD, so I have added an
open item. It would be good to figure out the root issue here, and I
am ready to bet that the problem is deeper th
On Tue, Mar 23, 2021 at 10:08 AM Amul Sul wrote:
> On Mon, Mar 22, 2021 at 3:03 PM Amit Langote
> wrote:
> >
> > On Mon, Mar 22, 2021 at 5:26 PM Amul Sul wrote:
> > > In heapam_relation_copy_for_cluster(), begin_heap_rewrite() sets
> > > rwstate->rs_new_rel->rd_smgr correctly but next line
> tu
On Mon, Mar 22, 2021 at 3:03 PM Amit Langote wrote:
>
> On Mon, Mar 22, 2021 at 5:26 PM Amul Sul wrote:
> > In heapam_relation_copy_for_cluster(), begin_heap_rewrite() sets
> > rwstate->rs_new_rel->rd_smgr correctly but next line
> > tuplesort_begin_cluster()
> > get called which cause the syste
On Mon, Mar 22, 2021 at 5:26 PM Amul Sul wrote:
> In heapam_relation_copy_for_cluster(), begin_heap_rewrite() sets
> rwstate->rs_new_rel->rd_smgr correctly but next line tuplesort_begin_cluster()
> get called which cause the system cache invalidation and due to CCA setting,
> wipe out rwstate->rs_
In heapam_relation_copy_for_cluster(), begin_heap_rewrite() sets
rwstate->rs_new_rel->rd_smgr correctly but next line tuplesort_begin_cluster()
get called which cause the system cache invalidation and due to CCA setting,
wipe out rwstate->rs_new_rel->rd_smgr which wasn't restored for the subsequent
Hello,
While executing the below test case server crashed with Segfault 11 on
master branch.
I have enabled the CLOBBER_CACHE_ALWAYS in src/include/pg_config_manual.h
Issue is only reproducing on master branch.
*Test Case:*
CREATE TABLE sm_5_323_table (col1 numeric);
CREATE INDEX sm_5_323_idx ON
34 matches
Mail list logo
