On Wednesday, September 23, 2020 2:37 PM, Tsunakawa, Takayuki wrote:
> > I revised the patch based from my understanding of Horiguchi-san's
> > comment, but I could be wrong.
> > Quoting:
> >
> > "
> > + /* Get the number of blocks for the supplied
> relation's
> > fork */
> > +
From: Amit Kapila
> The idea is that we can't use this optimization if the value is not
> cached because we can't rely on lseek behavior. See all the discussion
> between Horiguchi-San and me in the thread above. So, how would you
> ensure that if we don't use Kirk-San's proposal?
Hmm, buggy
From: Jamison, Kirk/ジャミソン カーク
> I revised the patch based from my understanding of Horiguchi-san's comment,
> but I could be wrong.
> Quoting:
>
> "
> + /* Get the number of blocks for the supplied relation's
> fork */
> + nblocks = smgrnblocks(smgr_reln,
On Wed, Sep 23, 2020 at 8:04 AM tsunakawa.ta...@fujitsu.com
wrote:
>
> From: Amit Kapila
> > > > > Don't
> > > > > we need to guarantee the cache to be valid while recovery?
> > > > >
> > > >
> > > > One possibility could be that we somehow detect that the value we
> > > > are using is cached
On Wed, Sep 23, 2020 at 7:56 AM tsunakawa.ta...@fujitsu.com
wrote:
>
> (3)
> if (reln->smgr_cached_nblocks[forknum] == blocknum)
> reln->smgr_cached_nblocks[forknum] = blocknum + 1;
> else
> + {
> + /*
> +* DropRelFileNodeBuffers
On Wednesday, September 23, 2020 11:26 AM, Tsunakawa, Takayuki wrote:
> I looked at v14.
Thank you for checking it!
> (1)
> + /* Get the total number of blocks for the supplied relation's
> fork */
> + for (j = 0; j < nforks; j++)
> + {
> +
From: Amit Kapila
> > > > Don't
> > > > we need to guarantee the cache to be valid while recovery?
> > > >
> > >
> > > One possibility could be that we somehow detect that the value we
> > > are using is cached one and if so then only do this optimization.
> >
> > I basically like this direction.
I looked at v14.
(1)
+ /* Get the total number of blocks for the supplied relation's
fork */
+ for (j = 0; j < nforks; j++)
+ {
+ BlockNumber block = smgrnblocks(smgr_reln,
forkNum[j]);
+ nblocks
On Wednesday, September 16, 2020 5:32 PM, Kyotaro Horiguchi wrote:
> At Wed, 16 Sep 2020 10:05:32 +0530, Amit Kapila
> wrote in
> > On Wed, Sep 16, 2020 at 9:02 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Wed, 16 Sep 2020 08:33:06 +0530, Amit Kapila
> > > wrote in
> > > > On Wed, Sep 16,
On Wed, Sep 16, 2020 at 2:02 PM Kyotaro Horiguchi
wrote:
>
> At Wed, 16 Sep 2020 10:05:32 +0530, Amit Kapila
> wrote in
> > On Wed, Sep 16, 2020 at 9:02 AM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Wed, 16 Sep 2020 08:33:06 +0530, Amit Kapila
> > > wrote in
> > > > On Wed, Sep 16, 2020 at
At Wed, 16 Sep 2020 10:05:32 +0530, Amit Kapila wrote
in
> On Wed, Sep 16, 2020 at 9:02 AM Kyotaro Horiguchi
> wrote:
> >
> > At Wed, 16 Sep 2020 08:33:06 +0530, Amit Kapila
> > wrote in
> > > On Wed, Sep 16, 2020 at 7:46 AM Kyotaro Horiguchi
> > > wrote:
> > By the way I'm not sure that
On Wed, Sep 16, 2020 at 9:02 AM Kyotaro Horiguchi
wrote:
>
> At Wed, 16 Sep 2020 08:33:06 +0530, Amit Kapila
> wrote in
> > On Wed, Sep 16, 2020 at 7:46 AM Kyotaro Horiguchi
> > wrote:
> > > Is this means lseek(SEEK_END) doesn't count blocks that are
> > > write(2)'ed (by smgrextend) but not
At Wed, 16 Sep 2020 08:33:06 +0530, Amit Kapila wrote
in
> On Wed, Sep 16, 2020 at 7:46 AM Kyotaro Horiguchi
> wrote:
> > Is this means lseek(SEEK_END) doesn't count blocks that are
> > write(2)'ed (by smgrextend) but not yet flushed? (I don't think so,
> > for clarity.) The nblocks cache is
On Wed, Sep 16, 2020 at 7:46 AM Kyotaro Horiguchi
wrote:
>
> At Wed, 2 Sep 2020 08:18:06 +0530, Amit Kapila
> wrote in
> > On Wed, Sep 2, 2020 at 7:01 AM Kyotaro Horiguchi
> > wrote:
> > > Isn't a relation always locked asscess-exclusively, at truncation
> > > time? If so, isn't even the
At Wed, 16 Sep 2020 11:56:29 +0900 (JST), Kyotaro Horiguchi
wrote in
(Oops! Some of my comments duplicate with Tsunakawa-san, sorry.)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Thanks for the new version. Jamison.
At Tue, 15 Sep 2020 11:11:26 +, "k.jami...@fujitsu.com"
wrote in
> Hi,
>
> > BTW, I think I see one problem in the code:
> > >
> > > if (RelFileNodeEquals(bufHdr->tag.rnode, rnode.node) &&
> > > + bufHdr->tag.forkNum == forkNum[j] && tag.blockNum >=
>
The code doesn't seem to be working correctly.
(1)
+ for (block_num = 0; block_num <= nblocks;
block_num++)
should be
+ for (block_num = firstDelBlock[fork_num];
block_num < nblocks; block_num++)
because:
* You only want to
At Wed, 2 Sep 2020 08:18:06 +0530, Amit Kapila wrote
in
> On Wed, Sep 2, 2020 at 7:01 AM Kyotaro Horiguchi
> wrote:
> > Isn't a relation always locked asscess-exclusively, at truncation
> > time? If so, isn't even the result of lseek reliable enough?
> >
>
> Even if the relation is locked,
Hi,
> BTW, I think I see one problem in the code:
> >
> > if (RelFileNodeEquals(bufHdr->tag.rnode, rnode.node) &&
> > + bufHdr->tag.forkNum == forkNum[j] && tag.blockNum >=
> > + bufHdr->firstDelBlock[j])
> >
> > Here, I think you need to use 'i' not 'j' for forkNum and
> > firstDelBlock as
On Tuesday, September 8, 2020 1:02 PM, Amit Kapila wrote:
Hello,
> On Mon, Sep 7, 2020 at 1:33 PM k.jami...@fujitsu.com
> wrote:
> >
> > On Wednesday, September 2, 2020 5:49 PM, Amit Kapila wrote:
> > > On Wed, Sep 2, 2020 at 9:17 AM Tom Lane wrote:
> > > >
> > > > Amit Kapila writes:
> > > > >
From: tsunakawa.ta...@fujitsu.com
> (1)
> + INIT_BUFFERTAG(newTag,
> rnode.node, forkNum[j], firstDelBlock[j]);
>
> And you need to use i here, too.
I remember the books "Code Complete" and/or "Readable Code" suggest to use
meaningful loop variable names
From: Amit Kapila
> if (RelFileNodeEquals(bufHdr->tag.rnode, rnode.node) &&
> + bufHdr->tag.forkNum == forkNum[j] &&
> + bufHdr->tag.blockNum >= firstDelBlock[j])
>
> Here, I think you need to use 'i' not 'j' for forkNum and
> firstDelBlock as those are arrays w.r.t forks. That might fix
On Mon, Sep 7, 2020 at 1:33 PM k.jami...@fujitsu.com
wrote:
>
> On Wednesday, September 2, 2020 5:49 PM, Amit Kapila wrote:
> > On Wed, Sep 2, 2020 at 9:17 AM Tom Lane wrote:
> > >
> > > Amit Kapila writes:
> > > > Even if the relation is locked, background processes like
> > > > checkpointer
On Wednesday, September 2, 2020 5:49 PM, Amit Kapila wrote:
> On Wed, Sep 2, 2020 at 9:17 AM Tom Lane wrote:
> >
> > Amit Kapila writes:
> > > Even if the relation is locked, background processes like
> > > checkpointer can still touch the relation which might cause
> > > problems. Consider a
On Wed, Sep 2, 2020 at 9:17 AM Tom Lane wrote:
>
> Amit Kapila writes:
> > Even if the relation is locked, background processes like checkpointer
> > can still touch the relation which might cause problems. Consider a
> > case where we extend the relation but didn't flush the newly added
> >
On Wednesday, September 2, 2020 10:31 AM, Kyotaro Horiguchi wrote:
> Hello.
>
> At Tue, 1 Sep 2020 13:02:28 +, "k.jami...@fujitsu.com"
> wrote in
> > On Tuesday, August 18, 2020 3:05 PM (GMT+9), Amit Kapila wrote:
> > > Today, again thinking about this point it occurred to me that during
> >
Amit Kapila writes:
> Even if the relation is locked, background processes like checkpointer
> can still touch the relation which might cause problems. Consider a
> case where we extend the relation but didn't flush the newly added
> pages. Now during truncate operation, checkpointer can still
On Wed, Sep 2, 2020 at 7:01 AM Kyotaro Horiguchi
wrote:
>
> Hello.
>
> At Tue, 1 Sep 2020 13:02:28 +, "k.jami...@fujitsu.com"
> wrote in
> > On Tuesday, August 18, 2020 3:05 PM (GMT+9), Amit Kapila wrote:
> > > Today, again thinking about this point it occurred to me that during
> > >
I'd like make a subtle correction.
At Wed, 02 Sep 2020 10:31:22 +0900 (JST), Kyotaro Horiguchi
wrote in
> By the way
>
> > #define BUF_DROP_THRESHOLD 500 /* NBuffers divided by 2 */
>
> NBuffers is not a constant. Even if we wanted to set the macro as
> described in the comment,
Hello.
At Tue, 1 Sep 2020 13:02:28 +, "k.jami...@fujitsu.com"
wrote in
> On Tuesday, August 18, 2020 3:05 PM (GMT+9), Amit Kapila wrote:
> > Today, again thinking about this point it occurred to me that during
> > recovery
> > we can reliably find the relation size and after Thomas's
On Tuesday, August 18, 2020 3:05 PM (GMT+9), Amit Kapila wrote:
> On Fri, Aug 7, 2020 at 9:33 AM Tom Lane wrote:
> >
> > Amit Kapila writes:
> > > On Sat, Aug 1, 2020 at 1:53 AM Andres Freund
> wrote:
> > >> We could also just use pg_class.relpages. It'll probably mostly be
> > >> accurate
On Fri, Aug 7, 2020 at 9:33 AM Tom Lane wrote:
>
> Amit Kapila writes:
> > On Sat, Aug 1, 2020 at 1:53 AM Andres Freund wrote:
> >> We could also just use pg_class.relpages. It'll probably mostly be
> >> accurate enough?
>
> > Don't we need the accurate 'number of blocks' if we want to
On Fri, Aug 7, 2020 at 11:03 PM Robert Haas wrote:
>
> On Fri, Aug 7, 2020 at 12:52 PM Tom Lane wrote:
> > At least in the case of segment zero, the file will still exist. It'll
> > have been truncated to zero length, and if the filesystem is stupid about
> > holes in files then maybe a write
On Fri, Aug 7, 2020 at 9:33 AM Tom Lane wrote:
>
> Amit Kapila writes:
> > On Sat, Aug 1, 2020 at 1:53 AM Andres Freund wrote:
> >> We could also just use pg_class.relpages. It'll probably mostly be
> >> accurate enough?
>
> > Don't we need the accurate 'number of blocks' if we want to
On Fri, Aug 7, 2020 at 12:52 PM Tom Lane wrote:
> At least in the case of segment zero, the file will still exist. It'll
> have been truncated to zero length, and if the filesystem is stupid about
> holes in files then maybe a write to a high block number would consume
> excessive disk space,
Robert Haas writes:
> On Fri, Aug 7, 2020 at 12:09 PM Tom Lane wrote:
>> ... it's not very clear what to do in
>> backends that haven't got access to that table, but maybe we could just
>> accept that backends that are forced to flush dirty buffers might do some
>> useless writes in such cases.
On Fri, Aug 7, 2020 at 12:09 PM Tom Lane wrote:
> As for (1), maybe we don't need to keep the info in shmem. I'll just
> point out that the checkpointer has *already got* a complete list of all
> recently-dropped relations, cf pendingUnlinks in sync.c. So you could
> imagine looking aside at
Robert Haas writes:
> On Fri, Aug 7, 2020 at 12:03 AM Tom Lane wrote:
>> Yeah, there is no room for "good enough" here. If a dirty buffer remains
>> in the system, the checkpointer will eventually try to flush it, and fail
>> (because there's no file to write it to), and then checkpointing will
On Fri, Aug 7, 2020 at 12:03 AM Tom Lane wrote:
> Yeah, there is no room for "good enough" here. If a dirty buffer remains
> in the system, the checkpointer will eventually try to flush it, and fail
> (because there's no file to write it to), and then checkpointing will be
> stuck. So we cannot
On Fri, Aug 07, 2020 at 10:08:23AM +0300, Konstantin Knizhnik wrote:
On 07.08.2020 00:33, Tomas Vondra wrote:
Unfortunately Konstantin did not share any details about what workloads
he tested, what config etc. But I find the "no regression" hypothesis
rather hard to believe, because we're
On Friday, August 7, 2020 12:38 PM, Amit Kapila wrote:
Hi,
> On Thu, Aug 6, 2020 at 6:53 AM k.jami...@fujitsu.com
> wrote:
> >
> > On Saturday, August 1, 2020 5:24 AM, Andres Freund wrote:
> >
> > Hi,
> > Thank you for your constructive review and comments.
> > Sorry for the late reply.
> >
> > >
On 07.08.2020 00:33, Tomas Vondra wrote:
Unfortunately Konstantin did not share any details about what workloads
he tested, what config etc. But I find the "no regression" hypothesis
rather hard to believe, because we're adding non-trivial amount of code
to a place that can be quite hot.
Amit Kapila writes:
> On Sat, Aug 1, 2020 at 1:53 AM Andres Freund wrote:
>> We could also just use pg_class.relpages. It'll probably mostly be
>> accurate enough?
> Don't we need the accurate 'number of blocks' if we want to invalidate
> all the buffers? Basically, I think we need to perform
On Sat, Aug 1, 2020 at 1:53 AM Andres Freund wrote:
>
> Hi,
>
> On 2020-07-31 15:50:04 -0400, Tom Lane wrote:
> > Andres Freund writes:
>
> > > Wonder if the temporary fix is just to do explicit hashtable probes for
> > > all pages iff the size of the relation is < s_b / 500 or so. That'll
> > >
On Fri, Aug 7, 2020 at 3:03 AM Tomas Vondra
wrote:
>
> >But I understand the sentiment on the added overhead on BufferAlloc.
> >Regarding the case where the patch would potentially affect workloads
> >that fit into RAM but not into shared buffers, could one of Andres'
> >suggested idea/s above
On Thu, Aug 6, 2020 at 6:53 AM k.jami...@fujitsu.com
wrote:
>
> On Saturday, August 1, 2020 5:24 AM, Andres Freund wrote:
>
> Hi,
> Thank you for your constructive review and comments.
> Sorry for the late reply.
>
> > Hi,
> >
> > On 2020-07-31 15:50:04 -0400, Tom Lane wrote:
> > > Andres Freund
On Thu, Aug 06, 2020 at 01:23:31AM +, k.jami...@fujitsu.com wrote:
On Saturday, August 1, 2020 5:24 AM, Andres Freund wrote:
Hi,
Thank you for your constructive review and comments.
Sorry for the late reply.
Hi,
On 2020-07-31 15:50:04 -0400, Tom Lane wrote:
> Andres Freund writes:
> >
On Saturday, August 1, 2020 5:24 AM, Andres Freund wrote:
Hi,
Thank you for your constructive review and comments.
Sorry for the late reply.
> Hi,
>
> On 2020-07-31 15:50:04 -0400, Tom Lane wrote:
> > Andres Freund writes:
> > > Indeed. The buffer mapping hashtable already is visible as a
Hi,
On 2020-07-31 15:50:04 -0400, Tom Lane wrote:
> Andres Freund writes:
> > Indeed. The buffer mapping hashtable already is visible as a major
> > bottleneck in a number of workloads. Even in readonly pgbench if s_b is
> > large enough (so the hashtable is larger than the cache). Not to speak
Andres Freund writes:
> Indeed. The buffer mapping hashtable already is visible as a major
> bottleneck in a number of workloads. Even in readonly pgbench if s_b is
> large enough (so the hashtable is larger than the cache). Not to speak
> of things like a cached sequential scan with a cheap qual
Hi,
On 2020-07-31 13:39:37 -0400, Tom Lane wrote:
> Robert Haas writes:
> > Unfortunately, I don't have time for detailed review of this. I am
> > suspicious that there are substantial performance regressions that you
> > just haven't found yet. I would not take the position that this is a
> >
Robert Haas writes:
> Unfortunately, I don't have time for detailed review of this. I am
> suspicious that there are substantial performance regressions that you
> just haven't found yet. I would not take the position that this is a
> completely hopeless approach, or anything like that, but
On Friday, July 31, 2020 2:37 AM, Konstantin Knizhnik wrote:
> 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
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 have tested this patch at various workloads and hardware
On Wednesday, July 29, 2020 4:55 PM, Konstantin Knizhnik wrote:
> On 17.06.2020 09:14, k.jami...@fujitsu.com wrote:
> > Hi,
> >
> > Since the last posted version of the patch fails, attached is a rebased
> > version.
> > Written upthread were performance results and some benefits and challenges.
On 17.06.2020 09:14, k.jami...@fujitsu.com wrote:
Hi,
Since the last posted version of the patch fails, attached is a rebased version.
Written upthread were performance results and some benefits and challenges.
I'd appreciate your feedback/comments.
Regards,
Kirk Jamison
As far as i
Hi,
Since the last posted version of the patch fails, attached is a rebased version.
Written upthread were performance results and some benefits and challenges.
I'd appreciate your feedback/comments.
Regards,
Kirk Jamison
v8-Optimize-dropping-of-relation-buffers-using-dlist.patch
Description:
On Wednesday, March 25, 2020 3:25 PM, Kirk Jamison wrote:
> As for the performance and how it affects the read-only workloads.
> Using pgbench, I've kept the overload to a minimum, less than 1%.
> I'll post follow-up results.
Here's the follow-up results.
I executed the similar tests from top of
Hi,
I know this might already be late at end of CommitFest, but attached
is the latest version of the patch. The previous version only includes buffer
invalidation improvement for VACUUM. The new patch adds the same
routine for TRUNCATE WAL replay.
In summary, this patch aims to improve the
On Tue, Feb 4, 2020 at 4:57 AM k.jami...@fujitsu.com
wrote:
> Kindly check the attached V6 patch.
> Any thoughts on this?
Unfortunately, I don't have time for detailed review of this. I am
suspicious that there are substantial performance regressions that you
just haven't found yet. I would not
Hi,
I have rebased the patch to keep the CFbot happy.
Apparently, in the previous patch there was a possibility of infinite loop
when allocating buffers, so I fixed that part and also removed some whitespaces.
Kindly check the attached V6 patch.
Any thoughts on this?
Regards,
Kirk Jamison
Hi,
I have updated the patch (v5).
I tried to reduce the lock waiting times by using spinlock
when inserting/deleting buffers in the new hash table, and
exclusive lock when doing lookup for buffers to be dropped.
In summary, instead of scanning the whole buffer pool in
shared buffers, we just
On Wed, Nov 13, 2019 4:20AM (GMT +9), Tomas Vondra wrote:
> On Tue, Nov 12, 2019 at 10:49:49AM +, k.jami...@fujitsu.com wrote:
> >On Thurs, November 7, 2019 1:27 AM (GMT+9), Robert Haas wrote:
> >> On Tue, Nov 5, 2019 at 10:34 AM Tomas Vondra
> >>
> >> wrote:
> >> > 2) This adds another
On Tue, Nov 12, 2019 at 10:49:49AM +, k.jami...@fujitsu.com wrote:
On Thurs, November 7, 2019 1:27 AM (GMT+9), Robert Haas wrote:
On Tue, Nov 5, 2019 at 10:34 AM Tomas Vondra
wrote:
> 2) This adds another hashtable maintenance to BufferAlloc etc. but
> you've only done tests /
On Thurs, November 7, 2019 1:27 AM (GMT+9), Robert Haas wrote:
> On Tue, Nov 5, 2019 at 10:34 AM Tomas Vondra
> wrote:
> > 2) This adds another hashtable maintenance to BufferAlloc etc. but
> > you've only done tests / benchmark for the case this optimizes. I
> > think we need to see a
On Tue, Nov 5, 2019 at 10:34 AM Tomas Vondra
wrote:
> 2) This adds another hashtable maintenance to BufferAlloc etc. but
> you've only done tests / benchmark for the case this optimizes. I
> think we need to see a benchmark for workload that allocates and
> invalidates lot of buffers.
Hi Kirk,
On Tue, Nov 05, 2019 at 09:58:22AM +, k.jami...@fujitsu.com wrote:
Hi,
Another one that I'd need feedback of is the use of new dlist operations
for this cached buffer list. I did not use in this patch the existing
Postgres dlist architecture (ilist.h) because I want to
Hi,
> Another one that I'd need feedback of is the use of new dlist operations
> for this cached buffer list. I did not use in this patch the existing
> Postgres dlist architecture (ilist.h) because I want to save memory space
> as much as possible especially when NBuffers become large. Both
201 - 268 of 268 matches
Mail list logo
