Hi,
On 2019-05-07 12:12:37 -0400, Tom Lane wrote:
> Why do you think there's no limit? We ordinarily do
> RelationGetNumberOfBlocks at least once per query on a table
Well, for the main fork. Which already could have shrunk below the size
that led the FSM to be created. And we only do
Andres Freund writes:
> On 2019-05-07 12:04:11 -0400, Tom Lane wrote:
>> I do not think sinval messaging is going to be sufficient to avoid that
>> problem. sinval is only useful to tell you about changes if you first
>> take a lock strong enough to guarantee that no interesting change is
>>
Hi,
On 2019-05-07 12:04:11 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2019-05-07 09:34:42 -0400, Tom Lane wrote:
> >> I'm inclined to wonder why bother with invals at all.
>
> > But when updating the free space for the first four blocks, we're going
> > to either have to do an
Andres Freund writes:
> On 2019-05-07 09:34:42 -0400, Tom Lane wrote:
>> I'm inclined to wonder why bother with invals at all.
> But when updating the free space for the first four blocks, we're going
> to either have to do an smgrexists() to check whether somebody
> concurrently created the
Hi,
On 2019-05-07 09:34:42 -0400, Tom Lane wrote:
> I'm inclined to wonder why bother with invals at all. The odds are
> quite good that no other backend will care (which, I imagine, is the
> reasoning behind why the original patch was designed like it was).
> A table that has a lot of
Amit Kapila writes:
> On Mon, May 6, 2019 at 8:57 PM Andres Freund wrote:
>> On 2019-05-06 11:10:15 -0400, Robert Haas wrote:
>>> I think it's legitimate to question whether sending additional
>>> invalidation messages as part of the design of this feature is a good
>>> idea.
>> I don't think
On Mon, May 6, 2019 at 8:57 PM Andres Freund wrote:
> On 2019-05-06 11:10:15 -0400, Robert Haas wrote:
>
> > I think it's legitimate to question whether sending additional
> > invalidation messages as part of the design of this feature is a good
> > idea. If it happens frequently, it could
On Mon, May 6, 2019 at 8:33 PM Tom Lane wrote:
>
> Andres Freund writes:
> > On 2019-05-05 18:55:30 +0530, Amit Kapila wrote:
> >> I understand that we have to take a call here shortly, but as there is
> >> a weekend so I would like to wait for another day to see if anyone
> >> else wants to
On Mon, May 6, 2019 at 12:18 PM Andres Freund wrote:
> > None of that addresses the question of the distributed cost of sending
> > more sinval messages. If you have a million little tiny relations and
> > VACUUM goes through and clears one tuple out of each one, it will be
> > spewing sinval
Hi,
On 2019-05-06 11:52:12 -0400, Robert Haas wrote:
> On Mon, May 6, 2019 at 11:27 AM Andres Freund wrote:
> > > I think it's legitimate to question whether sending additional
> > > invalidation messages as part of the design of this feature is a good
> > > idea. If it happens frequently, it
On Mon, May 6, 2019 at 12:05 PM Tom Lane wrote:
> Robert Haas writes:
> > ... I guess you could incur the overhead repeatedly if the relation starts
> > out at 1 block, grows to 4, is vacuumed back down to 1, lather, rinse,
> > repeat, but is that actually realistic?
>
> While I've not studied
Robert Haas writes:
> ... I guess you could incur the overhead repeatedly if the relation starts
> out at 1 block, grows to 4, is vacuumed back down to 1, lather, rinse,
> repeat, but is that actually realistic?
While I've not studied the patch, I assumed that once a relation has an
FSM it won't
On Mon, May 6, 2019 at 11:27 AM Andres Freund wrote:
> > I think it's legitimate to question whether sending additional
> > invalidation messages as part of the design of this feature is a good
> > idea. If it happens frequently, it could trigger expensive sinval
> > resets more often. I don't
Hi,
On 2019-05-06 11:10:15 -0400, Robert Haas wrote:
> I'm really surprised that the original design of this patch involved
> storing state in global variables. That seems like a pretty poor
> decision. This is properly per-relation information, and any approach
> like that isn't going to work
On Sun, May 5, 2019 at 9:25 AM Amit Kapila wrote:
> I understand that we have to take a call here shortly, but as there is
> a weekend so I would like to wait for another day to see if anyone
> else wants to share his opinion.
I haven't looked deeply into the issues with this patch, but it seems
Andres Freund writes:
> On 2019-05-05 18:55:30 +0530, Amit Kapila wrote:
>> I understand that we have to take a call here shortly, but as there is
>> a weekend so I would like to wait for another day to see if anyone
>> else wants to share his opinion.
> I still think that's the right course.
Hi,
On 2019-05-05 18:55:30 +0530, Amit Kapila wrote:
> On Sat, May 4, 2019 at 2:55 PM Amit Kapila wrote:
> > On Fri, May 3, 2019 at 2:14 PM Amit Kapila wrote:
> > I am fine going with option (a), that's why I have prepared a revert
> > patch, but I have a slight fear that the other option might
On Sat, May 4, 2019 at 2:55 PM Amit Kapila wrote:
>
> On Fri, May 3, 2019 at 2:14 PM Amit Kapila wrote:
> >
> > On Fri, May 3, 2019 at 11:43 AM John Naylor
> > wrote:
>
> Attached is a revert patch. John, can you please once double-check to
> ensure I have not missed anything?
>
> To
On Sat, May 4, 2019 at 5:25 PM Amit Kapila wrote:
> Attached is a revert patch. John, can you please once double-check to
> ensure I have not missed anything?
Looks complete to me.
--
John Naylorhttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA,
On Fri, May 3, 2019 at 2:14 PM Amit Kapila wrote:
>
> On Fri, May 3, 2019 at 11:43 AM John Naylor
> wrote:
>
> Fair enough. I think we have tried to come up with a patch for an
> alternative approach, but it needs time. I will revert this tomorrow.
>
Attached is a revert patch. John, can
On Fri, May 3, 2019 at 11:43 AM John Naylor wrote:
> On Thu, May 2, 2019 at 4:57 PM Amit Kapila wrote:
> > On Thu, May 2, 2019 at 12:39 PM John Naylor
> > wrote:
> > >
> > Can you please test/review?
>
> There isn't enough time. But since I already wrote some debugging
> calls earlier
On Thu, May 2, 2019 at 4:57 PM Amit Kapila wrote:
>
> On Thu, May 2, 2019 at 12:39 PM John Naylor
> wrote:
> >
> Okay, I have updated the patch to incorporate your changes and call
> relcache invalidation at required places. I have updated comments at a
> few places as well. The summarization
On Thu, May 2, 2019 at 12:39 PM John Naylor wrote:
>
> On Thu, May 2, 2019 at 2:31 PM Amit Kapila wrote:
> > @@ -776,7 +776,10 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
> > if ((rel->rd_smgr->smgr_fsm_nblocks == 0 ||
> > rel->rd_smgr->smgr_fsm_nblocks == InvalidBlockNumber) &&
> >
On Thu, May 2, 2019 at 2:31 PM Amit Kapila wrote:
> @@ -776,7 +776,10 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
> if ((rel->rd_smgr->smgr_fsm_nblocks == 0 ||
> rel->rd_smgr->smgr_fsm_nblocks == InvalidBlockNumber) &&
> !smgrexists(rel->rd_smgr, FSM_FORKNUM))
> + {
>
On Tue, Apr 30, 2019 at 11:42 AM John Naylor
wrote:
>
> On Fri, Apr 26, 2019 at 11:52 AM Amit Kapila wrote:
> > As discussed above, we need to issue an
> > invalidation for following points: (a) when vacuum finds there is no
> > FSM and page has more space now, I think you can detect this in
>
On Thu, May 2, 2019 at 7:36 AM John Naylor wrote:
>
> On Wed, May 1, 2019 at 11:24 PM Andres Freund wrote:
> >
> > Hi,
> >
> > On 2019-04-18 14:10:29 -0700, Andres Freund wrote:
> > > My compromise suggestion would be to try to give John and Amit ~2 weeks
> > > to come up with a cleanup
On Thu, May 2, 2019 at 3:42 AM Alvaro Herrera wrote:
>
> On 2019-May-01, Amit Kapila wrote:
>
> > On Tue, Apr 30, 2019 at 7:52 PM Alvaro Herrera
> > wrote:
>
> > > Hmm ... so, if vacuum runs and frees up any space from any of the pages,
> > > then it should send out an invalidation -- it
On Wed, May 1, 2019 at 11:24 PM Andres Freund wrote:
>
> Hi,
>
> On 2019-04-18 14:10:29 -0700, Andres Freund wrote:
> > My compromise suggestion would be to try to give John and Amit ~2 weeks
> > to come up with a cleanup proposal, and then decide whether to 1) revert
> > 2) apply the new patch,
On 2019-May-01, Amit Kapila wrote:
> On Tue, Apr 30, 2019 at 7:52 PM Alvaro Herrera
> wrote:
> > Hmm ... so, if vacuum runs and frees up any space from any of the pages,
> > then it should send out an invalidation -- it doesn't matter what the
> > FSM had, just that there is more free space
On Wed, May 1, 2019 at 09:08:54AM -0700, Andres Freund wrote:
> So sure, there's a few typo fixes, one bugfix, and one buildfarm test
> stability issue. Doesn't seem crazy for a nontrivial improvement.
OK, my ignorant opinion was just based on the length of discussion
threads.
--
Bruce
Hi,
On 2019-05-01 11:28:11 -0400, Bruce Momjian wrote:
> On Wed, May 1, 2019 at 08:24:25AM -0700, Andres Freund wrote:
> > Hi,
> >
> > On 2019-04-18 14:10:29 -0700, Andres Freund wrote:
> > > My compromise suggestion would be to try to give John and Amit ~2 weeks
> > > to come up with a cleanup
On Wed, May 1, 2019 at 08:24:25AM -0700, Andres Freund wrote:
> Hi,
>
> On 2019-04-18 14:10:29 -0700, Andres Freund wrote:
> > My compromise suggestion would be to try to give John and Amit ~2 weeks
> > to come up with a cleanup proposal, and then decide whether to 1) revert
> > 2) apply the new
Hi,
On 2019-04-18 14:10:29 -0700, Andres Freund wrote:
> My compromise suggestion would be to try to give John and Amit ~2 weeks
> to come up with a cleanup proposal, and then decide whether to 1) revert
> 2) apply the new patch, 3) decide to live with the warts for 12, and
> apply the patch in
On Wed, May 1, 2019 at 11:43 AM Amit Kapila wrote:
>
> On Tue, Apr 30, 2019 at 7:52 PM Alvaro Herrera
> wrote:
> > but that seems correct.
> > Sounds better than keeping outdated entries indicating no-space-available.
>
> Agreed, but as mentioned in one of the above emails, I am also bit
>
On Wed, May 1, 2019 at 9:57 AM John Naylor wrote:
>
> On Tue, Apr 30, 2019 at 12:48 PM Amit Kapila wrote:
> >
> > On Fri, Apr 26, 2019 at 10:46 AM John Naylor
> > wrote:
> > I don't much like the new function name GetAlternatePage, may be
> > GetPageFromLocalFSM or something like that. OTOH, I
On Tue, Apr 30, 2019 at 12:48 PM Amit Kapila wrote:
>
> On Fri, Apr 26, 2019 at 10:46 AM John Naylor
> wrote:
> >
> > On Wed, Apr 17, 2019 at 2:04 AM Andres Freund wrote:
> > >
> > > Hi,
> > >
> > > I'm somewhat unhappy in how much the no-fsm-for-small-rels exposed
> > > complexity that looks
On Tue, Apr 30, 2019 at 6:06 PM Amit Kapila wrote:
>
> On Tue, Apr 30, 2019 at 2:24 PM Dilip Kumar wrote:
> >
> > insert into atacc1 values (21, 22, 23);
> > +ERROR: could not read block 0 in file "base/16384/31379": read only
> > 0 of 8192 bytes
> >
> > I have analysed this failure. Seems
On Tue, Apr 30, 2019 at 7:52 PM Alvaro Herrera wrote:
>
> On 2019-Apr-30, John Naylor wrote:
>
> > On Fri, Apr 26, 2019 at 11:52 AM Amit Kapila
> > wrote:
> > > As discussed above, we need to issue an
> > > invalidation for following points: (a) when vacuum finds there is no
> > > FSM and page
On 2019-Apr-30, John Naylor wrote:
> On Fri, Apr 26, 2019 at 11:52 AM Amit Kapila wrote:
> > As discussed above, we need to issue an
> > invalidation for following points: (a) when vacuum finds there is no
> > FSM and page has more space now, I think you can detect this in
> >
On Fri, Apr 26, 2019 at 11:52 AM Amit Kapila wrote:
> As discussed above, we need to issue an
> invalidation for following points: (a) when vacuum finds there is no
> FSM and page has more space now, I think you can detect this in
> RecordPageWithFreeSpace
I took a brief look and we'd have to
On Fri, Apr 26, 2019 at 11:52 AM Amit Kapila wrote:
>
> On Thu, Apr 25, 2019 at 12:39 PM John Naylor
> wrote:
> >
> > On Wed, Apr 24, 2019 at 1:58 PM Amit Kapila wrote:
> > >
> >
> > Sorry for not noticing earlier, but this patch causes a regression
> > test failure for me (attached)
> >
>
>
On Wed, Apr 17, 2019 at 2:04 AM Andres Freund wrote:
>
> Hi,
>
> I'm somewhat unhappy in how much the no-fsm-for-small-rels exposed
> complexity that looks like it should be purely in freespacemap.c to
> callers.
I took a stab at untying the free space code from any knowledge about
heaps, and
On Thu, Apr 25, 2019 at 12:39 PM John Naylor
wrote:
>
> On Wed, Apr 24, 2019 at 1:58 PM Amit Kapila wrote:
> >
>
> Sorry for not noticing earlier, but this patch causes a regression
> test failure for me (attached)
>
Can you please try to finish the remaining work of the patch (I am bit
tied
On Wed, Apr 24, 2019 at 1:58 PM Amit Kapila wrote:
>
Sorry for not noticing earlier, but this patch causes a regression
test failure for me (attached)
--
John Naylorhttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Apr 24, 2019 at 1:58 PM Amit Kapila wrote:
> The two improvements in this code which are discussed in this thread
> and can be done independently to this patch are:
> a. use one bit to represent each block in the map. This gives us the
> flexibility to use the map for the different
On Wed, Apr 24, 2019 at 9:49 PM Andres Freund wrote:
> On 2019-04-24 11:28:32 +0530, Amit Kapila wrote:
> > On Tue, Apr 23, 2019 at 10:59 PM Andres Freund wrote:
> > > > > On 2019-04-22 18:49:44 +0530, Amit Kapila wrote:
> > > > I think we should first try to see in this new scheme (a) when to
Hi,
On 2019-04-24 11:28:32 +0530, Amit Kapila wrote:
> On Tue, Apr 23, 2019 at 10:59 PM Andres Freund wrote:
> > > > On 2019-04-22 18:49:44 +0530, Amit Kapila wrote:
> > > I think we should first try to see in this new scheme (a) when to set
> > > the map, (b) when to clear it, (c) how to use.
On Tue, Apr 23, 2019 at 10:59 PM Andres Freund wrote:
> > > On 2019-04-22 18:49:44 +0530, Amit Kapila wrote:
> > > > /*
> > > > @@ -1132,9 +1110,6 @@ fsm_local_set(Relation rel, BlockNumber
> > > > cur_nblocks)
> > > > /* Set the status of the cached target block to 'unavailable'. */
> >
Hi,
On 2019-04-23 13:31:25 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2019-04-23 15:46:17 +0530, Amit Kapila wrote:
> >> If we invalidate it only when there's no space on the page, then when
> >> should we set it back to available, because if we don't do that, then
> >> we might miss
Andres Freund writes:
> On 2019-04-23 15:46:17 +0530, Amit Kapila wrote:
>> If we invalidate it only when there's no space on the page, then when
>> should we set it back to available, because if we don't do that, then
>> we might miss the space due to concurrent deletes.
> Well, deletes don't
Hi,
On 2019-04-23 15:46:17 +0530, Amit Kapila wrote:
> On Mon, Apr 22, 2019 at 10:34 PM Andres Freund wrote:
> > On 2019-04-22 18:49:44 +0530, Amit Kapila wrote:
> > > /*
> > > @@ -1132,9 +1110,6 @@ fsm_local_set(Relation rel, BlockNumber cur_nblocks)
> > > BlockNumber blkno,
> > >
On Mon, Apr 22, 2019 at 10:34 PM Andres Freund wrote:
> On 2019-04-22 18:49:44 +0530, Amit Kapila wrote:
> > /*
> > @@ -1132,9 +1110,6 @@ fsm_local_set(Relation rel, BlockNumber cur_nblocks)
> > BlockNumber blkno,
> > cached_target_block;
> >
> > - /* The
Hi,
On 2019-04-22 18:49:44 +0530, Amit Kapila wrote:
> Attached is a hacky and work-in-progress patch to move fsm map to
> relcache. This will give you some idea. I think we need to see if
> there is a need to invalidate the relcache due to this patch. I think
> some other comments of Andres
On Fri, Apr 19, 2019 at 2:46 PM Amit Kapila wrote:
>
>
> > > I am thinking that we should at least give it a try to move the map to
> > > rel cache level to see how easy or difficult it is and also let's wait
> > > for a day or two to see if Andres/Tom has to say anything about this
> > > or on
On Fri, Apr 19, 2019 at 1:17 PM John Naylor wrote:
> On Fri, Apr 19, 2019 at 10:38 AM Amit Kapila wrote:
> >
> > I think if we go this route, then
> > we might need to revisit it in the future to optimize it, but maybe
> > that is the best alternative as of now.
>
> It's a much lighter-weight
On Fri, Apr 19, 2019 at 10:38 AM Amit Kapila wrote:
>
> On Thu, Apr 18, 2019 at 2:10 PM John Naylor
> wrote:
> > Agreed. I suspect the most realistic way to address most of the
> > objections in a short amount of time would be to:
> >
> > 1. rip out the local map
> > 2. restore hio.c to only
On April 18, 2019 7:53:58 PM PDT, Tom Lane wrote:
>Amit Kapila writes:
>> I am thinking that we should at least give it a try to move the map
>to
>> rel cache level to see how easy or difficult it is and also let's
>wait
>> for a day or two to see if Andres/Tom has to say anything about this
Amit Kapila writes:
> I am thinking that we should at least give it a try to move the map to
> rel cache level to see how easy or difficult it is and also let's wait
> for a day or two to see if Andres/Tom has to say anything about this
> or on the response by me above to improve the current
On Thu, Apr 18, 2019 at 2:10 PM John Naylor wrote:
>
> On Thu, Apr 18, 2019 at 2:48 PM Amit Kapila wrote:
> > I respect and will follow whatever will be the consensus after
> > discussion. However, I request you to wait for some time to let the
> > discussion conclude. If we can't get to an
>
Andres Freund writes:
> My compromise suggestion would be to try to give John and Amit ~2 weeks
> to come up with a cleanup proposal, and then decide whether to 1) revert
> 2) apply the new patch, 3) decide to live with the warts for 12, and
> apply the patch in 13. As we would already have a
Hi,
On 2019-04-17 12:20:29 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2019-04-17 15:49:29 +0530, Amit Kapila wrote:
> >> OTOH, if we want to extend it later for whatever reason to a relation
> >> level cache, it shouldn't be that difficult as the implementation is
> >> mostly
On Thu, Apr 18, 2019 at 2:48 PM Amit Kapila wrote:
> I respect and will follow whatever will be the consensus after
> discussion. However, I request you to wait for some time to let the
> discussion conclude. If we can't get to an
> agreement or one of John or me can't implement what is
On Wed, Apr 17, 2019 at 9:50 PM Tom Lane wrote:
>
> Andres Freund writes:
> > On 2019-04-17 15:49:29 +0530, Amit Kapila wrote:
> >> OTOH, if we want to extend it later for whatever reason to a relation
> >> level cache, it shouldn't be that difficult as the implementation is
> >> mostly
On Wed, Apr 17, 2019 at 9:46 PM Andres Freund wrote:
>
> Hi,
>
> On 2019-04-17 15:49:29 +0530, Amit Kapila wrote:
> > > *and* stash the bitmap of
> > > pages that we think are used/free as a bitmap somewhere below the
> > > relcache.
> >
> > I think maintaining at relcache level will be tricky
Andres Freund writes:
> On 2019-04-17 15:49:29 +0530, Amit Kapila wrote:
>> OTOH, if we want to extend it later for whatever reason to a relation
>> level cache, it shouldn't be that difficult as the implementation is
>> mostly contained in freespace.c (fsm* functions) and I think the
>> relation
Hi,
On 2019-04-17 15:49:29 +0530, Amit Kapila wrote:
> > *and* stash the bitmap of
> > pages that we think are used/free as a bitmap somewhere below the
> > relcache.
>
> I think maintaining at relcache level will be tricky when there are
> insertions and deletions happening in the small
Hi,
On 2019-04-17 13:09:05 +0800, John Naylor wrote:
> On Wed, Apr 17, 2019 at 2:04 AM Andres Freund wrote:
> >
> > Hi,
> >
> > I'm somewhat unhappy in how much the no-fsm-for-small-rels exposed
> > complexity that looks like it should be purely in freespacemap.c to
> > callers.
> >
> >
> >
On Wed, Apr 17, 2019 at 10:39 AM John Naylor
wrote:
> On Wed, Apr 17, 2019 at 2:04 AM Andres Freund wrote:
>
> > +/* Only create the FSM if the heap has greater than this many blocks */
> > +#define HEAP_FSM_CREATION_THRESHOLD 4
> >
> > Hm, this seems to be tying freespace.c closer to heap than
On Wed, Apr 17, 2019 at 2:04 AM Andres Freund wrote:
>
> Hi,
>
> I'm somewhat unhappy in how much the no-fsm-for-small-rels exposed
> complexity that looks like it should be purely in freespacemap.c to
> callers.
>
>
> extern Size GetRecordedFreeSpace(Relation rel, BlockNumber heapBlk);
>
Andres Freund writes:
> On 2019-04-16 14:31:25 -0400, Tom Lane wrote:
>> This can only work at all if an inaccurate map is very fail-soft,
>> which I'm not convinced it is
> I think it better needs to be fail-soft independent of this the no-fsm
> patch. Because the fsm is not WAL logged etc,
Hi,
On 2019-04-16 14:31:25 -0400, Tom Lane wrote:
> Andres Freund writes:
> > I'm kinda thinking that this is the wrong architecture.
>
> The bits of that patch that I've looked at seemed like a mess
> to me too. AFAICT, it's trying to use a single global "map"
> for all relations (strike 1)
Andres Freund writes:
> I'm kinda thinking that this is the wrong architecture.
The bits of that patch that I've looked at seemed like a mess
to me too. AFAICT, it's trying to use a single global "map"
for all relations (strike 1) without any clear tracking of
which relation the map currently
Hi,
I'm somewhat unhappy in how much the no-fsm-for-small-rels exposed
complexity that looks like it should be purely in freespacemap.c to
callers.
extern Size GetRecordedFreeSpace(Relation rel, BlockNumber heapBlk);
-extern BlockNumber GetPageWithFreeSpace(Relation rel, Size spaceNeeded);
73 matches
Mail list logo
